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

ArC TCKs #31589

Merged
merged 5 commits into from
Mar 8, 2023
Merged

ArC TCKs #31589

merged 5 commits into from
Mar 8, 2023

Conversation

Ladicek
Copy link
Contributor

@Ladicek Ladicek commented Mar 3, 2023

Related to #28558

  • add Arquillian adapter for ArC
  • add CDI TCK runner (with an exclusion list that is expected to shrink over time)
  • add AtInject TCK runner (passing as is)
  • add CDI Lang Model TCK runner (passing as is)

@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Mar 3, 2023
@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 3, 2023

One thing that is perhaps not very nice is that the independent-projects/arc directory becomes a bit cluttered with all the TCK stuff. Maybe all the TCKs should be put into a separate independent-projects/arc/tck directory?

@Ladicek Ladicek requested review from manovotn and mkouba March 3, 2023 15:09
@gsmet
Copy link
Member

gsmet commented Mar 3, 2023

Just a suggestion, but given it is somehow part of the MP effort, maybe we could have the TCK parts in tcks/ (but I'm not sure the current parent would work for you)?

@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 3, 2023

I will eventually add a CDI TCK runner with full Quarkus into tcks. What I'm adding here is a TCK with standalone ArC (which runs in 20 seconds; the full Quarkus variant will likely take minutes).

Maybe I should add both at the same time, but that requires keeping 2 exclusion lists in sync or some way to reuse the same exclusion list on both places.

@quarkus-bot

This comment has been minimized.

@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 6, 2023

The sheer number of failures on Windows is because

2023-03-03T15:51:44.9329971Z java.io.UncheckedIOException: java.nio.file.FileSystemException:
D:\a\quarkus\quarkus\independent-projects\arc\cdi-tck-runner\target\ArcArquillian6670968999252800138\app\libraries\arc-cdi-tck-porting-pkg-999-SNAPSHOT.jar:
The process cannot access the file because it is being used by another process

I admit I never tried the Arquillian adapter on Windows :-) Will take a look.

Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming we get the CI to pass, the PR LGTM.
Finally, we won't have to work in two repos to check and fix TCK failures 👍

@manovotn
Copy link
Contributor

manovotn commented Mar 6, 2023

The JDK 17+ failure is also related.

org.jboss.cdi.tck.tests.definition.stereotype.StereotypeDefinitionTest.testMultipleStereotypesAllowed line 83

The failure happens when the TCK asserts that a set of qualifiers (Set<Annotation>) contains a certain annotation literal.
The literal is there, implemented as _ArcAnnotationLiteral (with equals method that I'd presume should work), but the assertion fails. This works fine in JDK 11.
Funnily enough, using the exact same literal, we can perform typesafe resolution just fine (literally one line above in the test), so the problem lies somewhere in combination of newer JDK, invoking Set#contains() and our custom annotation literal impl? 🤔

@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 6, 2023

I have a fix for the Windows failures. I'll look at the StereotypeDefinitionTest tomorrow, if @manovotn doesn't beat me to it.

@manovotn
Copy link
Contributor

manovotn commented Mar 6, 2023

I have a fix for the Windows failures. I'll look at the StereotypeDefinitionTest tomorrow, if @manovotn doesn't beat me to it.

Didn't manage to figure that out before I went AFK today, we can sync on it tomorrow.

Apparently, a lot of people use the construct `new AnnotationLiteral<MyAnnotation>() {}`
to create an annotation literal for a memberless annotation. This is wrong, because
the result doesn't implement the annotation interface, as specified by the contract
of the `Annotation.equals()` method:

> Returns true if the specified object represents an annotation that is logically
> equivalent to this one. In other words, returns true if the specified object
> is an instance of the same annotation interface as this instance, all of whose
> members are equal to the corresponding member of this annotation, as defined below:
> [...]

Yet, this occurs both in ArC tests and in the CDI TCK. This commit therefore
improves the generated `equals()` method of ArC annotation literals for this
particular case. If the annotation type has no members, the generated `equals()`
method no longer verifies that the other object's class implements the annotation
interface. Instead, it verifies that the other object's class implements
`java.lang.annotation.Annotation` and that the `annotationType()` classes
are equal. This is also what CDI's `AnnotationLiteral` does.
@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 7, 2023

Both problems should be fixed now:

  1. For the StereotypeDefinitionTest, I added a commit that improves equality of ArC's generated annotation literals. This improvement is a slight deviation from the Annotation.equals() contract to accommodate for an edge case that AnnotationLiteral also supports -- creating an annotation literal for a memberless annotation without implementing the annotation interface.
  2. For the Windows failures, I improved the Arquillian adapter to prevent file handle leaks caused by JarURLConnection. This is a notorious issue with a well known solution -- prevent URLConnection "caching" for the jar protocol.

@manovotn manovotn added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 7, 2023
@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 7, 2023

The JDK 17 Windows failure is the same as in #31602:

Error:  io.quarkus.arc.test.config.UnremovedConfigMappingTest  Time elapsed: 0.563 s  <<< ERROR!
java.lang.RuntimeException: java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:689)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$12(ClassBasedTestDescriptor.java:395)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:395)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:211)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:84)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:148)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:147)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:127)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:90)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:55)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:102)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:114)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:86)
	at org.junit.platform.launcher.core.DefaultLauncherSession$DelegatingLauncher.execute(DefaultLauncherSession.java:86)
	at org.apache.maven.surefire.junitplatform.LazyLauncher.execute(LazyLauncher.java:55)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.execute(JUnitPlatformProvider.java:223)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invokeAllTests(JUnitPlatformProvider.java:175)
	at org.apache.maven.surefire.junitplatform.JUnitPlatformProvider.invoke(JUnitPlatformProvider.java:139)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:456)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:169)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:595)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:581)
Caused by: java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
	at io.quarkus.runtime.Application.start(Application.java:101)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.runner.bootstrap.StartupActionImpl.run(StartupActionImpl.java:273)
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:642)
	... 43 more
Caused by: jakarta.enterprise.inject.spi.DeploymentException: Configuration validation failed:
	java.util.NoSuchElementException: SRCFG00014: The config property mapping.prop is required but it could not be found in any config source
	at io.quarkus.arc.runtime.ConfigRecorder.registerConfigProperties(ConfigRecorder.java:83)
	at io.quarkus.deployment.steps.ConfigBuildStep$registerConfigClasses1377682816.deploy_0(Unknown Source)
	at io.quarkus.deployment.steps.ConfigBuildStep$registerConfigClasses1377682816.deploy(Unknown Source)
	... 51 more
Caused by: io.smallrye.config.ConfigValidationException: Configuration validation failed:
	java.util.NoSuchElementException: SRCFG00014: The config property mapping.prop is required but it could not be found in any config source
	at io.smallrye.config.ConfigMappingProvider.mapConfigurationInternal(ConfigMappingProvider.java:985)
	at io.smallrye.config.ConfigMappingProvider.lambda$mapConfiguration$3(ConfigMappingProvider.java:941)
	at io.smallrye.config.SecretKeys.doUnlocked(SecretKeys.java:29)
	at io.smallrye.config.ConfigMappingProvider.mapConfiguration(ConfigMappingProvider.java:941)
	at io.smallrye.config.ConfigMappings.mapConfiguration(ConfigMappings.java:91)
	at io.smallrye.config.ConfigMappings.mapConfiguration(ConfigMappings.java:87)
	at io.smallrye.config.ConfigMappings.registerConfigProperties(ConfigMappings.java:43)
	at io.quarkus.arc.runtime.ConfigRecorder.registerConfigProperties(ConfigRecorder.java:81)
	... 53 more

So this clearly is not caused by this PR, but needs looking at. I'll see if I can figure it out, but Config is scary.

@quarkus-bot
Copy link

quarkus-bot bot commented Mar 7, 2023

Failing Jobs - Building 603ecfa

Status Name Step Failures Logs Raw logs
✔️ Gradle Tests - JDK 11
Gradle Tests - JDK 11 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 11
✔️ JVM Tests - JDK 17
JVM Tests - JDK 17 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 19
Native Tests - Security1 Build ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Gradle Tests - JDK 11 Windows #

📦 integration-tests/gradle

io.quarkus.gradle.devmode.BasicKotlinApplicationModuleDevModeTest.main line 19 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

io.quarkus.gradle.devmode.JandexMultiModuleProjectDevModeTest.main line 21 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

⚙️ JVM Tests - JDK 17 Windows #

- Failing: extensions/arc/deployment 
! Skipped: devtools/cli extensions/agroal/deployment extensions/amazon-lambda-http/deployment and 428 more

📦 extensions/arc/deployment

io.quarkus.arc.test.config.UnremovedConfigMappingTest. - More details - Source on GitHub

java.lang.RuntimeException: java.lang.RuntimeException: Failed to start quarkus
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:689)
	at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$12(ClassBasedTestDescriptor.java:395)

@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 8, 2023

All the failures are known flaky tests, merging this.

@Ladicek Ladicek merged commit a2463b3 into quarkusio:main Mar 8, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 8, 2023
@quarkus-bot quarkus-bot bot added this to the 3.0 - main milestone Mar 8, 2023
@Ladicek Ladicek deleted the arc-tck branch March 8, 2023 08:55
@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 8, 2023

When it comes to UnremovedConfigMappingTest, I can't reproduce it locally (ran more than 1500x), and I don't see anything obvious in the code :-(

@mkouba
Copy link
Contributor

mkouba commented Mar 9, 2023

Maybe all the TCKs should be put into a separate independent-projects/arc/tck directory?

+10

@Ladicek
Copy link
Contributor Author

Ladicek commented Mar 9, 2023

I'll move them in some next PR.

@mkouba
Copy link
Contributor

mkouba commented Mar 9, 2023

I'll move them in some next PR.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants