Re: [PR] SOLR-17617: Add missing Inject to CollectionProperty and InstallCoreData [solr]
colvinco commented on PR #3023: URL: https://github.com/apache/solr/pull/3023#issuecomment-2659978985 @gerlowskija fyi, here's what I've done so far. https://github.com/apache/solr/blob/1e3f4afb23761b9462eb5577f3b286fa1b0bc998/solr/core/src/test/org/apache/solr/handler/admin/api/V2APISmokeTest.java Most of the cases are simple to write but some things need extra setup and I don't know the APIs myself so progress is a bit slow. I'm sure someone else who knows what's what here could finish it off more quickly... But I guess the main thing is whether the general approach is reasonable and worth continuing in this fashion. The assertions are simply checking for a successful response code, rather than making direct assertions on the effect of the request, or the content of thet response. I have found several found several issues so far (which I will raise JIRAs for) so I think it's definitely providing some value, and adding new cases as new APIs are written should be trivial enough. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17617: Add missing Inject to CollectionProperty and InstallCoreData [solr]
colvinco commented on PR #3023: URL: https://github.com/apache/solr/pull/3023#issuecomment-2592917253 I'm happy to write them in Java. @epugh on the dev-thread said > Is there some way to programmatically look up each v2 API and then issue a request? 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). 1. Use the `/_introspect` endpoints to drive it 2. Do some code analysis (e.g. use reflection) to drive it 3. Generate tests (or stubs) from the OpenAPI spec - I see OAG is already here https://github.com/apache/solr/blob/6d838cb3de9774e1a17208a78210f8968ce4e959/solr/solrj/build.gradle#L131 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). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-17617: Add missing Inject to CollectionProperty and InstallCoreData [solr]
gerlowskija commented on PR #3023: URL: https://github.com/apache/solr/pull/3023#issuecomment-2583508494 @colvinco asked the following in a dev-thread related to this PR: > I've not really looked at the BATS tests before. [...] I take it I can just run > curl to poke each API. So they should be quick enough to write when I > get back to this next week. Is that the best option then? Or should I use > the RestTestBase and go that route instead? 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
[PR] SOLR-17617: Add missing Inject to CollectionProperty and InstallCoreData [solr]
colvinco opened a new pull request, #3023: URL: https://github.com/apache/solr/pull/3023 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: - [ ] I have reviewed the guidelines for [How to Contribute](https://github.com/apache/solr/blob/main/CONTRIBUTING.md) and my code conforms to the standards described there to the best of my ability. - [ ] I have created a Jira issue and added the issue ID to my pull request title. - [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation) - [ ] I have developed this patch against the `main` branch. - [ ] I have run `./gradlew check`. - [ ] I have added tests for my changes. - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
