TestNG issues

[JanWesterkamp-iJUG]

There where open issues with TestNG.

@brunobat and @Emily-Jiang, I might missed something, but currently we are using JUnit 5, right? And the idea is to migrate to TestNG, because all od MircoProfile is using it?
And the intended migration to Test NG fails?

Originally I thought, TestNG creates some issues while using JUnit 5 - but I am wrong here?
I would definitive recommend (keep) using JUnit 5 instead TestNG 7...

Either way, I can give some recipe to overwrite the outdated dependencies from the current microprofile-parent 3.0.
Basically there are two options, one short (but not recomended!) version and the longer version, that might reduce furure breaking changes.

But first, can you clarify the exact issue with TestNG, please?

[brunobat]

Tests fail due to missing injection. See Jasmin's message from Set 13 on Gitter, please

[JanWesterkamp-iJUG]

Tests fail due to missing injection. See Jasmin's message from Set 13 on Gitter, please

Do you have a link to that message? I don not use Gitter... ;-)

[Emily-Jiang]

@JanWesterkamp-iJUG here is the error message

Cannot invoke "org.eclipse.microprofile.telemetry.tracing.tck.exporter.InMemorySpanExporter.reset()" because "this.spanExporter" is null

java.lang.NullPointerException: Cannot invoke "org.eclipse.microprofile.telemetry.tracing.tck.exporter.InMemorySpanExporter.reset()" because "this.spanExporter" is null
at org.eclipse.microprofile.telemetry.tracing.tck.rest.RestSpanTest.setUp(RestSpanTest.java:73)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

[Emily-Jiang]

By the way, we are nearly out of time. If this issue cannot be solved today, we have to address this issue in the next release as we need to do the final RC asap (I plan to do a RC this evening), so that it will appear in maven central tomorrow morning.

[JanWesterkamp-iJUG]

The short version for a try could be using the current instead of outdated versions for the parent test dependency definitions, (where somehow TestNG is included):

In the maven properties section of the tck (or the telemetry-tracing) module, add the following (as needed/used):

<version.testng>7.6.1</version.testng>
<version.junit>4.13.2</version.junit>
<version.hamcrest>1.3</version.hamcrest>
<version.arquillian>1.7.0.Alpha12</version.arquillian>

Especially that Arquillian 1.7.0.Alpha12 version fixes some TestNG 7 issues.
If we do not use TestNG at all, than an exclude might help, where it occurs unintendently.

Updating the JUnit 5 version BOM to the current one would be a good idea too:

<version.junit-jupiter>5.9.1</version.junit-jupiter>

Of course, the dependencies need to be configured too, not only by importing the BOM.

The long version for dependency overwrites would be using names for Maven properties that follow Maven naming conventions, which requires to add the dependencies in the dependency management section using that new name for the definition - If not usesed that way, my planed refacoring of the microprofile-parent property names will result in a breaking change for sure.

[Emily-Jiang]

The short version for a try could be using the current instead of outdated versions for the parent test dependency definitions, (where somehow TestNG is included):

In the maven properties section of the tck (or the telemetry-tracing) module, add the following (as needed/used):

<version.testng>7.6.1</version.testng> <version.junit>4.13.2</version.junit> <version.hamcrest>1.3</version.hamcrest> <version.arquillian>1.7.0.Alpha12</version.arquillian>

Especially that Arquillian 1.7.0.Alpha12 version fixes some TestNG 7 issues. If we do not use TestNG at all, than an exclude might help, where it occurs unintendently.

Updating the JUnit 5 version BOM to the current one would be a good idea too:

<version.junit-jupiter>5.9.1</version.junit-jupiter>

Of course, the dependencies need to be configured too, not only by importing the BOM.

The long version for dependency overwrites would be using names for Maven properties that follow Maven naming conventions, which requires to add the dependencies in the dependency management section using that new name for the definition - If not usesed that way, my planed refacoring of the microprofile-parent property names will result in a breaking change for sure.

Please go ahead to do a PR in this repo so that Yasmin can test it out asap.

[JanWesterkamp-iJUG]

By the way, we are nearly out of time. If this issue cannot be solved today, we have to address this issue in the next release as we need to do the final RC asap (I plan to do a RC this evening), so that it will appear in maven central tomorrow morning.

@Emily-Jiang: My questions above not aswered yet.

If everything works fine with JUnit 5 (instead of TestNG), we can (and should stop) here immediately from my perspective.
If JUnit 5 TCK usage generates problems because of TestNG, we need to proceed and fix the issue BEFORE we do a final release of MP Telemetry Tracing 1.0.

And the last is valid regarding Otel Java implementation and Spec support the way of doing the configuration (rely on Otel stable config property with deviating behaviour in MP, as decided by the majority of the MPWG) - and that fact was not my idea...

We have also deviations for proper semver versioning of a lot of MP specs in the MP 6.0 Plan doing breaking changes with service releases too (Jakakta EE 10 & Java SE 11)...

So having a vendor offering to act with is implementation as CI and pushing things forward is very important and welcome, but MP will fail in general, if jobs are done half only and technical dept piles up - this is in fact slowing us down and even more important let users decide to use something else!

[Emily-Jiang]

By the way, we are nearly out of time. If this issue cannot be solved today, we have to address this issue in the next release as we need to do the final RC asap (I plan to do a RC this evening), so that it will appear in maven central tomorrow morning.

@Emily-Jiang: My questions above not aswered yet.

If everything works fine with JUnit 5 (instead of TestNG), we can (and should stop) here immediately from my perspective. If JUnit 5 TCK usage generates problems because of TestNG, we need to proceed and fix the issue BEFORE we do a final release of MP Telemetry Tracing 1.0.

The answer is yes.

[JanWesterkamp-iJUG]

By the way, we are nearly out of time. If this issue cannot be solved today, we have to address this issue in the next release as we need to do the final RC asap (I plan to do a RC this evening), so that it will appear in maven central tomorrow morning.

@Emily-Jiang: My questions above not aswered yet.
If everything works fine with JUnit 5 (instead of TestNG), we can (and should stop) here immediately from my perspective. If JUnit 5 TCK usage generates problems because of TestNG, we need to proceed and fix the issue BEFORE we do a final release of MP Telemetry Tracing 1.0.

The answer is yes.

Just to be sure: The problem is not exising in the main branch with JUnit 5?
And is only a problem related to this PR banch: #41?

If your answer is still yes, then to outdated test dependency versions could/should be addressed in the microprofile-parent - I creaded an PR for that there already: microprofile/microprofile-parent#67

The only left thing then is to do the remaining version updates, i.e. found be the dependabot already - I will create a seaparte issue here for that aspect now.

[JanWesterkamp-iJUG]

@JanWesterkamp-iJUG here is the error message

Cannot invoke "org.eclipse.microprofile.telemetry.tracing.tck.exporter.InMemorySpanExporter.reset()" because "this.spanExporter" is null

java.lang.NullPointerException: Cannot invoke "org.eclipse.microprofile.telemetry.tracing.tck.exporter.InMemorySpanExporter.reset()" because "this.spanExporter" is null
at org.eclipse.microprofile.telemetry.tracing.tck.rest.RestSpanTest.setUp(RestSpanTest.java:73)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
...

If the problem is related to JUnit 5, then updating might fix it (see linked release notes in this PR created by dependabot - so I would suggest to do that update anyway.

[JanWesterkamp-iJUG]

To be sure that this is the root cause, I would need additional information to the error, but some of the fixed bugs might returned null before wrongly.