Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to provide test instances as CDI beans #233

Open
manovotn opened this issue Jan 31, 2025 · 1 comment
Open

Attempt to provide test instances as CDI beans #233

manovotn opened this issue Jan 31, 2025 · 1 comment
Milestone

Comments

@manovotn
Copy link
Collaborator

manovotn commented Jan 31, 2025

Based on several issues over the years, I wanted to try and rewrite Weld JUnit 5 extension so that it can provide test instances as CDI beans.
Following are my observations, gains, limitations etc...

First of all, here's my here's my branch - https://github.com/manovotn/weld-testing/tree/provideTestInstance
The diff against current master ( past 5.0.0.Beta1) can be seen here - https://github.com/weld/weld-testing/compare/master...manovotn:weld-testing:provideTestInstance?expand=1

Limitations:

  • All @WeldSetup WeldInitiator fields need to be static
    • We now provide the instance so the configuration needs to be readable before we instantiate that class
  • JUnit requires that we provide instances before we even know if the test is a @Nested one
    • All Weld configuration must be known in the outermost class; anything declared on a @Nested test is ignored as we already have a running container at that point
    • Nested tests cannot declare WeldInitiator at all - configuration is taken from the outermost class
      • You simply cannot have static field inside a nested class anyway
    • Similarly for @EnableAutoWeld, annotations declared on nested classes won't be detected before the container is running
      • Among other things, we now have to perform whole discovery in automagic mode once we get information about the outermost class. It is no longer safe to assume that inner classes don't inherit some of the beans from outer classes (such as in this test)
  • WeldJunitEnricher would have a breaking change
    • enrich(Class<?> testClass, ExtensionContext context, Weld weld, Builder weldInitiatorBuilder);
    • The first parameter can no longer be Objest testInstance but instead needs to be just the Class<?>
    • This shouldn't be a problem for the most part but it is still a breaking change
  • Parallel execution of tests is more limited - given when we start and stop container, we cannot support a scenario in which test methods execute in parallel
    • The linked commit has a change to junit-platform.properties showing that - we can still execute classes in parallel at least

Gains:

Other notes:

  • For debugging, I highly recommend turning off parallel execution in junit-platform.properties file
  • Several tests in the linked branch are still failing (just run mvn clean install on the branch) and as far as I know we cannot make those work - mostly some variations on @Nested tests
    • This is also a breaking change for we most likely have users running those (based on having tests for these specific cases)
  • My branch currently runs with the following results:
    • [ERROR] Tests run: 136, Failures: 5, Errors: 4, Skipped: 1
@mkouba
Copy link
Member

mkouba commented Feb 7, 2025

I'm not really sure if it's worth the hassle. I mean, the use cases mentioned in the "Gains" section seem to be quite niche, i.e. intercepting test methods and modifying the test AnnotatedType. And IMO the gains do not justify the breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants