-
Notifications
You must be signed in to change notification settings - Fork 679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SOLR-17617: Add missing Inject to CollectionProperty and InstallCoreData #3023
base: main
Are you sure you want to change the base?
Conversation
@colvinco asked the following in a dev-thread related to this PR:
My two cents are that we'd be better sticking to a Java-based test here. BATS tests give you, in some senses, a bit more fidelity than a JUnit test since they're testing the same tgz or zip distribution that an end-user downloads, but that comes at a pretty steep cost: they're much much slower to run, can't be parallelized, aren't cross-platform, etc. So my suggestion would be to write in Java unless you run into a reason to do otherwise. That said - if you like the simplicity of BATS or think the extra fidelity is worth it, I'm OK with that too. This'll be a big boost to our v2 test coverage, so it'll be a huge win whichever approach you take. |
I'm happy to write them in Java. @epugh on the dev-thread said
Off the top of my head, I guess there's three approaches to that sort of thing (there could well be others or more refined takes on these).
I think the problem with 1 and 2 is that something dynamic like that is probably quite hard to unpick if something does go wrong, e.g. if it misses a set of endpoints will it be obvious? Plus if any endpoints have precondition (collection existing) then it would still need extra context to drive it. The problem with 3 is how you manage the generated source and whether you're able to generate something that's ready to use, or if it's just generating stubs that require manual intervention to turn into usable tests. If it's the former then it's great, because changes to the spec can automatically generate updated tests. With the latter it can be a good starting point for writing tests normally. ATM that is what I think I will do - use the spec to generate an initial set of test stubs so that I cover all the existing endpoints, but I'm not planning on putting the generator into the PR (unless it is in state of producing ready to use output anyway). |
Having said that, I've started down the road of just doing it manually for now... solr/solr/core/src/test/org/apache/solr/handler/admin/api/V2APISmokeTest.java Lines 63 to 86 in 238dd55
|
https://issues.apache.org/jira/browse/SOLR-17617
Description
Add missing
@Inject
annotations.Tests
TODO: add a smoke test for all v2 APIs
Checklist
Please review the following and check all that apply:
main
branch../gradlew check
.