Hi Chris!
> [ not a review ]
[feedback is always welcome, especially from you]
> I think that the approach is reasonable (to inject ’test’ into the package name between the reverse DNS prefix and the specific logical technology suffix).
It's actually '.tests'... Robert (Muir) suggested .test as well but I
must have added that extra 's' somewhere along the line and I didn't
have the heart to redo all the refactorings...
> I’ve played a little with similar in the Elasticsearch codebase - but not yet advanced it to a point that eliminates all split packages.
ES is much larger and more complex, so I'm not surprised. Same like
Solr. Lucene is relatively simple -- lots of code but all of the
modules are fairly simple structure-wise. Makes such experiments more
approachable.
> TestSecrets also seems reasonable. It’s a pity that it intrudes somewhat on the actual product code, but not to any extent that would be concerning.
This is quite funny - I actually reengineered the internal-package
trick, although my version looked slightly different (it had public
methods returning the "secret" accessors, although they couldn't be
instantiated by anything else than the internal package - you can see
it in the commit history... then I recalled that the JDK had a similar
solution and peeked at how that was done. I thought it was nicer as it
didn't pollute the API. So now you know whom I borrowed the idea from
- you. :)
The only difference is the use of unsafe to make sure classes have
their static blocks invoked. I didn't want to bring unsafe into the
mix and used Class.forName(apiClazz.getName()). I don't think this can
have any side-effects whatsoever and the contract on this variant of
forName has the initialization flag on, so it seems to be safe -
correct me if I'm wrong though.
> It’s great that you can now have a test_framework module. And, from what I can see, the moduleXXX configurations that you introduced recently work just fine for the Gradle dependency configuration.
I have to admit this works rather nicely. There are some rough corners
we discovered already but I think they're all fixable with relative
easy -
https://issues.apache.org/jira/browse/LUCENE-10328 I'm sure in the end this could be extracted into a more reusable
gradle plugin, probably replacing the built-in support for modules,
but for the time being it's just easier to work with those separate
gradle scripts.
> Once this is in, then it will be possible to patch area specific unit tests into the actual product module and `require` the framework, right?
Yes, I think this is doable. It's what Uwe has been asking for - his
panama branch currently requires tricks that shouldn't be there.
> And if we had that, it’s not a big leap to maybe refactor some of the secrets to be injected too ( but I accept that that is not really necessary either, and I’m not sure how the IDEs or Gradle would like this )
What secrets do you have in mind here? As for IDEs: IntelliJ should
work fine as it converts each source set into a logical dependency
unit. I don't think how Eclipse can be made to digest all this - for
now, we create a big bag of sources without submodule demarcation. So
it compiles, although is far from ideal.
Dawid
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org