Hi David,

> On 20 Dec 2021, at 17:12, Dawid Weiss <[email protected]> wrote:
> 
> [snip]
> 
>> 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...

Ah yes, '.tests’ - which is also fine.

> [ snip ]
>> 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.

Right, if you need access to package-privates from an “API” package, 
then secrets are a fine tool for the job.

> 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.

You are correct (you’re safe). Lucene has bumped the minimum JDK
to 17, right? If so, you could use MethodHandles.Lookup::ensureInitialized.
Or this could be done separately, if there is interest. (hmm.. maybe you
don’t wanna go down this rabbit hole now!, lookup access could be an
issue )

>> 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.

Yeah, it seems so.

>> 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.

Ok, great. I think we’re on the same track.

>> 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?

Nothing specific, just that if we’re injecting code into the module
(patching the module for unit testing purposes), then even the secrets
(currently in the product code), could be effectively injected too.
(The code that sets things up in the API packages). Almost as if each
module had its own mini-test-framework, which could be (but does not
strictly need to be) part of its unit test. 

> 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.

I was mostly thinking of how the IDE would react to —-patch-module, and
whether or not it could handle things like auto-complete, etc, from
consuming code whose view point just sees the patched module as one
complete unit. 

Anyway, that is a distraction from the good work here, and something for
another day.

-Chris.



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to