[
https://issues.apache.org/jira/browse/SOLR-11032?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16219590#comment-16219590
]
Hoss Man commented on SOLR-11032:
---------------------------------
I have some concerns about how the current patch works...
* {{solr/solrj/src/test/org/apache/solr/example/}} should probably be something
like {{solr/solrj/src/test/org/apache/solr/client/ref-guide-examples/}} ?
** makes sense to keep these tests under {{client}} since that's what they test
** we already have other pre-existing "example" tests that relate specifically
to the {{techproducts}} example configs, so it would be good to help
disambiguate these
* some choices in {{RefGuideSolrJExampleTest}} don't make a lot of sense to
me...
** why use the {{streaming}} configset?
*** particularly since {{using-solrj.adoc}} says things like {{The following
snippet uses a SolrClient to query Solr's "techproducts" example collection}}
*** as things stand: someone could go make drastic changes to the schema in
{{sample_techproducts_configs}} that would cause these examples/documentation
to break, and this "test" won't actually catch that problem
** why use 4 nodes and a repFactor of 2 ... if nothing in the docs/tests
actually care or refer to this?
** in general, there doesn't really seem to be many assertions that the things
we document happening actaully happen.
*** Example: {{queryWithRawSolrParamsExample()}} ... this test could index a
few docs prior to the start {{tag}}, and then show in the included snippet
accessing the fields specified in the {{fl}} (rather then just {{toString}}
each {{SolrDocument}} ... and _after_ the {{end}}, the test could assert that
the expected number of docs was found and that each doc had the expected field
values
*** Likewise {{queryWithSolrQueryExample()}} ... this test has code to execute
a query and loop over the results, but at no point are any SolrInputDocuments
added to the index, and that code isn't even included in the {{tag}} snippet
include in the doc -- so what's the point of executing the query? why not jsut
assert that the {{SolrQuery}} object has the expected {{SolrParams}} ? (or add
some SolrInputDocuments and assert that the QueryResponse contains the expected
results
** we should avoid unneccessary {{System.out.println}}
*** i understand the value of having those when the code is shown in the doc --
but the doc would probably be just as useful/readabile if the code simply said
{{print(...)}} where that is a new No-Op static function we define in the test
*** better still: our {{print(String)}} function could be setup as a mock to
actually assert that we get the output we expect in the order expected...{code}
private static Queue<String> printExpects = new ArrayDeque();
private static expectPrint(String... expected)
assertTrue(printExpects.isEmpty(), "Left over expected strings!");
printExpects = new ArrayDeque<>(expected);
}
@After
public static checkNoLeftoverPrintsExpected() {
expectPrint();
}
private static print(String value) {
String expected = printExpects.poll();
assertNotNull(expected, "Asked to print " + value + " but nothing expected.")
assertTrue(value.contains(expected), "Asked to print " + value + " but it
does not contain the next expected substring: " + expected);
}
public void testSomething() {
...
expectPrint("bar", "baz");
// tag::solrj-query-something[]
...
final QueryResponse response = client.query("techproducts", query);
for(SolrDocument result : response.getResults()) {
print(result.getFieldValue());
}
// end::solrj-query-something[]
...
{code}
* I _really_ don't like that the build.xml now copies files into
{{solr/solr-ref-guide/src/}}
** that seems like a recipe for disaster/confusion as people working on these
docs see that {{using-solrj.adoc}} includes those files and try to edit them
directly.
** I understand that jekyll needs all the files it operates on to be in the
same "work dir" -- but that's why the {{solr/solr-ref-guide/build.xml}} file
already creates {{solr/build/solr-ref-guide/content/}} and uses it when running
jekyll
*** that's existed for a while because have templatized config files we need to
pre-process before running jekyll
*** IMHO we should *only* copy the {{*.java}} files we need to copy into this
directory, instead of into {{solr/solr-ref-guide/src/}}
** I appreciate the value of enabling "native" asciidoctor editors to properly
"preview" these files w/the includes -- but considering that even with this
patch, the person editing the docs will already having to know enough about the
ant build system to ensure {{ant copy-examples}} gets run before they
edit/preview their changes ... i just don't think this "cure" is better then
the original disease
*** particularly since they'll have to run it every time they want to
tweak/change/ammend the original {{*.java}} files and preview that the included
snippets look correct in their preview .. unless of course they accidently
forget about the {{*.java}} coping, and directly edit the files in
{{solr/solr-ref-guide/src/example}} (or perhaps do it intentionally as a way to
"quick check" their changes but then forget to change the files "for real" in
{{solr/solrj/...}}) in which case their preview will look fine, but as soon as
they run {{ant build-site}} or {{ant build-pdf}} their changes will get blown
away by the _original_ files from {{solr/solrj/....}
** I would suggest the patch just update {{build-init}} to copy the neccessary
{{*.java}} files to {{${build.content.dir\}}} and table discussion of how to
make these docs easier to preview in GUI editors to a new jira
*** Even if jekyll doesn't have any options to look at multiple dirs, perhaps
some of these asciidoc editors do? Perhaps, much like our {{dev-tools}} files
for IntelliJ & eclipse, there are config options/files we could setup so that
those work better?
* regardless of what decision is made regarding {{build.xml}} changes to copy
{{*.java}} files around to be used as include files, the {{meta-docs}} should
be updated to explain what's happening (and probably give a quick explanation
of the {{include}} syntax in general)
> Update solrj tutorial
> ---------------------
>
> Key: SOLR-11032
> URL: https://issues.apache.org/jira/browse/SOLR-11032
> Project: Solr
> Issue Type: Task
> Security Level: Public(Default Security Level. Issues are Public)
> Components: documentation, SolrJ, website
> Reporter: Karl Richter
> Attachments: SOLR-11032.patch, SOLR-11032.patch, SOLR-11032.patch
>
>
> The [solrj tutorial](https://wiki.apache.org/solr/Solrj) has the following
> issues:
> * It refers to 1.4.0 whereas the current release is 6.x, some classes are
> deprecated or no longer exist.
> * Document-object-binding is a crucial feature [which should be working in
> the meantime](https://issues.apache.org/jira/browse/SOLR-1945) and thus
> should be covered in the tutorial.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
