[
https://issues.apache.org/jira/browse/SOLR-13794?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chris M. Hostetter updated SOLR-13794:
--------------------------------------
Attachment: SOLR-13794.patch
SOLR-13794_code_only.patch
Status: Open (was: Open)
FWIW: The original motivation for the duplication seems to have come from this
comment/suggestion from shalin (emphasis added by me)...
https://issues.apache.org/jira/browse/SOLR-10272?focusedCommentId=16064813&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16064813
{quote}Since our collection creation API now uses _default configset by
default, our tests should also do the same if no explicit configset name is
specified. The _default for tests should be identical to the ones that users
will use. This ensures that if a functionality is tested using the _default
configset, it will actually work in the hands of our users. _If we need to
duplicate _default in two places to achieve this, then we should add a test to
assert that both are same._ This only really affects new tests since I am
assuming existing ones have been cut over to "conf" already.
{quote}
In a discussion about ensuring that {{_default}} was used correctly as the
"default configset" when solr was rurning in tests, shalin suggested that *if*
duplication was necessary then we should have a test verifying that the copies
were identical – but i can find no discussion indicating duplication *is*
necessary.
----
The attached patch leverages the existing {{ExternalPaths.DEFAULT_CONFIGSET}}
hueristicly determined path for the default configset and uses that to set the
{{solr.default.confdir}} _unless it is already set by the test environment_ (so
that external users leveraging the test-framework in their own tests can set it
as they see fit)
It also updates the existing
{{TestConfigSetsAPI.testUserAndTestDefaultConfigsetsAreSame()}} replacing the
existing logic for comparing the two directories (which is now nonsense since
there is only one directory) with a much simpler test to ensure that the value
used by {{ZkController}} when bootstraping matches the
{{ExternalPaths.DEFAULT_CONFIGSET}} – something that should (now) be garunteed
when running solr-core tests.
All tests pass with these changes, the only nocommit's in the current patch
relate to the removal of {{ZkController.getDefaultConfigDirFromClasspath(...)}}
which is how solr currently locates the duplicate
{{solr/core/src/test-files/solr/configsets/_default}} (by counting on
{{solr/core/src/test-files}} being the the classpath, and looking for
{{solr/configsets/_default}}). I think this is safe to remove, but I suppose
it's possible that end users have come to depend on this undocumented feature
of looking for this specific path in the classpath?
[~ichattopadhyaya] & [~shalinmangar]
* Do you see anything wrong with this approach?
* is there some motivation for this duplication that I'm overlooking?
* do you have any thoughts/concerns regarding the nocommits?
—
(NOTE: i've actually attached 2 patches – one includes all the changes; for
readability the second is just the code change subset of the first, w/o the
"noise" of {{git rm -r solr/core/src/test-files/solr/configsets/_default}})
> Delete solr/core/src/test-files/solr/configsets/_default
> --------------------------------------------------------
>
> Key: SOLR-13794
> URL: https://issues.apache.org/jira/browse/SOLR-13794
> Project: Solr
> Issue Type: Test
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Chris M. Hostetter
> Priority: Major
> Attachments: SOLR-13794.patch, SOLR-13794_code_only.patch
>
>
> For as long as we've had a {{_default}} configset in solr, we've also had a
> copy of that default in {{core/src/test-files/}} - as well as a unit test
> that confirms they are identical.
> It's never really been clear to me *why* we have this duplication, instead of
> just having the test-framework take the necessary steps to ensure that
> {{server/solr/configsets/_default}} is properly used when running tests.
> I'd like to propose we eliminate the duplication since it only ever seems to
> cause problems (notably spurious test failures when people modify the
> {{_default}} configset w/o remembering that they need to make identical edits
> to the {{test-files}} clone) and instead have {{SolrTestCase}} set the
> (already existing & supported) {{solr.default.confdir}} system property to
> point to the (already existing) {{ExternalPaths.DEFAULT_CONFIGSET}}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
