uschindler commented on pull request #571: URL: https://github.com/apache/lucene/pull/571#issuecomment-1005827412
> Please take a look at this comment/ chart, Uwe. https://issues.apache.org/jira/browse/LUCENE-10328?focusedCommentId=17468676&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17468676 > > How tests are run depends on the combination of whether source and test source sets are modules. > > We do not have module patching. I don't think it's possible to configure gradle internal infrastructure and plugins to reasonably use it. OK thanks for the picture. Much more information. I was hoping it is like that. That module patching does not work was known to me, I just stumbled on some support methods for it and I did not understand when they are used. I am perfectly fine when we run our tests for a class in classpath mode, although the main sourceset is modular. Technically this won't change the test results, as UNIT tests should only check internal assertions. Of course in reality it is more complicated. What I understood and which is missing in the image: The depenendecies are always modular, so lucene.core is put on module-path, even so we are running tests in classpath mode. This is waht this PR mainly changes, correct? reviously it was not fully working unless you explicitely declared it. What we should now do (after this is merged): - Review implementation vs api dependencies (both on gradle and on module-info). With my other PR for test-random-chains i found an issue because of this. E.g. the phonetic module uses commons-codec also in its public API. Compilation of my module worked for some reason, but forbiddenapis failed, as it was not able to see the classes (when inspecting the method signatures). Which is understandable. Also ICU needs to refer to ICU in an API (gradle) / transitive (module-system) way. So we should enable the exports checks. When developing the last patch about logging in core, i would make java.logging still non-transitive, because it is unlikely that you would use it in downstream code (although theres a public signature using JUL). Because of that I added a `SuppressWarnings("exports")` on the class using it. - Fix up the module-decriptor files to figure out that all is sane Uwe -- 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: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
