[jira] [Updated] (SOLR-13794) Delete solr/core/src/test-files/solr/configsets/_default

["Chris M. Hostetter (Jira)" <ji...@apache.org>]

     [ 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: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org