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

CDI-lite test update strategy #273

Closed
starksm64 opened this issue Aug 31, 2021 · 7 comments
Closed

CDI-lite test update strategy #273

starksm64 opened this issue Aug 31, 2021 · 7 comments
Assignees

Comments

@starksm64
Copy link
Contributor

starksm64 commented Aug 31, 2021

Based on today's CDI call, we want to revert #256, and then redo it in smaller stages based on small blocks of packages of tests after first applying the cdi-full group to tests based on:

We also want to move the cdi-full tests into a new package to separate them from the lite tests as this regrouping is done. The issue for that is #272.

@starksm64 starksm64 self-assigned this Aug 31, 2021
@starksm64
Copy link
Contributor Author

I'll work on the revision #256 and cherry picking of subsequent commits today.

starksm64 added a commit that referenced this issue Sep 1, 2021
…eans, #255"

Part 1 of #273, revert the following commits:
70c93c0
5c46c2d

Signed-off-by: Scott M Stark <starksm64@gmail.com>
starksm64 added a commit that referenced this issue Sep 8, 2021
…275)

* Add cdi-full group to passivation tests and move to full top package
Addresses #264, #273

Signed-off-by: Scott M Stark <starksm64@gmail.com>
starksm64 added a commit that referenced this issue Sep 21, 2021
* More decorator related tests need the cdi-full test group
Addresses #290, #273

Signed-off-by: Scott M Stark <starksm64@gmail.com>
@manovotn
Copy link
Contributor

Note that we didn't make an issue for specialization tests. I tried addressing that as I went over #289 but a few tests might have slipped through. It should be easy enough to catch once MN starts executing their Lite impl though (which presumably lacks specialization support).

Then there is the question of injection of BeanManager into tons of tests and also into the test super class (AbstractTest).
We would either have to change it everywhere or we could simply allow BeanManager injection even in Lite but restrict the set of usable methods (which is what I would vote for) - users are used to BeanManager anyway, so it might be a common attempt to inject it even in Lite.

@starksm64
Copy link
Contributor Author

Thanks @manovotn, when I pulled in the changes into a clone of cdi-tck upstream and ran against the weld core tck-runner, I'm seeing one error:

[ERROR] Tests run: 1205, Failures: 1, Errors: 0, Skipped: 2, Time elapsed: 46.857 s <<< FAILURE! - in TestSuite
[ERROR] org.jboss.cdi.tck.tests.full.inheritance.specialization.simple.broken.noextend1.SpecializingBeanImplementsInterfaceOnly.arquillianBeforeClass  Time elapsed: 0.051 s  <<< FAILURE!
java.lang.RuntimeException: Expected exception of type jakarta.enterprise.inject.spi.DefinitionException during deployment of _DEFAULT_, but no exception was thrown.

Add a dependent qualifier to Donkey_Broken fixed the issue.

@manovotn
Copy link
Contributor

Interesting, I even double checked that and always had all tests passing. Don't know how come it didn't fail for me. The change you made seems correct

@manovotn
Copy link
Contributor

manovotn commented Sep 25, 2021

@starksm64 hmm, this test is not executed for me (which is why I missed it) when I do mvn clean verify and I cannot seem to figure out why.
When I ran it explicitly (-Dtest=..) then it indeed fails because it missed the @Dependent annotation and it should be CDI_FULL group.

Did you execute those tests in any other way? Or do you have any other extra commit changing some setup?
I can see other tests in full package executed perfectly well, just this one is somehow left out...

EDIT: If I change the name of the test from SpecializingBeanImplementsInterfaceOnly to SpecializingBeanImplementsInterfaceOnlyTest, it suddenly starts being executed with just mvn clean verify. Still no clue as to why...

@starksm64
Copy link
Contributor Author

starksm64 commented Sep 25, 2021

Ok, strange. I was using the embedded profile jboss-tck-runner with the tck-core-suite.xml, so I had added that to the surefire configuration:

+                            <suiteXmlFiles>
+                                <suiteXmlFile>target/suites/tck-core-suite.xml</suiteXmlFile>
+                            </suiteXmlFiles>

That is the difference. TestNG will scan all classes for annotations while without this surefire is just scanning the dependencies for *Test classes.

@manovotn
Copy link
Contributor

Hmm, so the default profile in Weld doesn't have this applied automatically? I need to remember to look at that on Mon...
But you are right that this indeed looks like the cause, thanks for the hint!

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