bdelacretaz commented on pull request #5: URL: https://github.com/apache/sling-org-apache-sling-junit-core/pull/5#issuecomment-732048957
> Tests for the @TestReference(String filter) are run via SlingAnnotationsTestRunner [0] IIUC this means those tests are not executed automatically by the build? I don't think they are really useful then, but the existing test coverage is poor, there are no tests for the `AnnotationsProcessor` which is there the bulk of your changes are. Wouldn't it be better to create unit tests for the `AnnotationsProcessor`, before your changes, and then modify those tests to demonstrate the changes? Not sure how best to proceed given the poor existing coverage but I don't like the "embedded" tests that do not run as part of the build and remain in the built bundle (although we might have that in other modules, unfortunately). Another option is to create full-blown integration tests with a test-specific bundle, that's not too hard with the tools that were created in the meantime. And those integration tests can be in the same Git repository using the maven-invoker plugin as in https://github.com/apache/sling-org-apache-sling-adapter-annotations - I'm happy to provide pointers to similar test than what you need if desired. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
