NPEs in `JUnitOutputListenerProvider` #8433

jglick · 2025-04-18T00:00:27Z

I consistently get an error in NB 25 when testing Jenkins plugins with mvnd

java.lang.NullPointerException: Cannot invoke "org.netbeans.modules.maven.api.output.OutputVisitor$Context.getCurrentProject()" because the return value of "org.netbeans.modules.maven.api.output.OutputVisitor.getContext()" is null
	at org.netbeans.modules.maven.junit.JUnitOutputListenerProvider.sequenceEnd(JUnitOutputListenerProvider.java:593)
	at org.netbeans.modules.maven.execute.AbstractOutputHandler.processEnd(AbstractOutputHandler.java:219)
	at org.netbeans.modules.maven.execute.CommandLineOutputHandler$Output.run(CommandLineOutputHandler.java:371)
	at org.openide.util.RequestProcessor$Task.run(RequestProcessor.java:1403)
	at org.netbeans.modules.openide.util.GlobalLookup.execute(GlobalLookup.java:45)
	at org.openide.util.lookup.Lookups.executeWith(Lookups.java:287)
[catch] at org.openide.util.RequestProcessor$Processor.run(RequestProcessor.java:2018)

which seems to be similar to, yet distinct from, #5645. Can be reproduced by opening e.g. https://github.com/jenkinsci/workflow-job-plugin, selecting a @Test method, and running it, when the Maven executable is set to https://github.com/apache/maven-mvnd rather than regular Maven. The build gets as far as printing

…
--- hpi:3.61:test-runtime (default-test-runtime) @ workflow-job ---
Setting jenkins.addOpens to --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED
Setting jenkins.insaneHook to --patch-module='java.base=…/workflow-job-plugin/target/patch-modules/org-netbeans-insane-hook.jar' --add-exports=java.base/org.netbeans.insane.hook=ALL-UNNAMED

and then this exception is thrown and the output window is abruptly marked complete. With patch (tested using ant -f java/maven.junit run), the rest of the output starting with

--- surefire:3.5.2:test (default-cli) @ workflow-job ---
Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider

-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running org.jenkinsci.plugins.workflow.job.WorkflowRunTest
=== Starting durationIsCalculated(org.jenkinsci.plugins.workflow.job.WorkflowRunTest)
…

is printed normally.

Note that fixing the NPE reported above was not enough; I just got another NPE. Reviewing the file, I found a general lack of respect for @CheckForNull for two method calls so I tried to fix all occurrences just in case. I do not recall details of what this Context is (it has no Javadoc).

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

are all your own work, and you have the right to contribute them.
are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

Was this PR correctly labeled, did the right tests run? When did they run?
Is this PR squashed?
Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

lkishalmi

@jglick The change seems to be fine. Could you rebase this one on delivery so this could get into NB26-rc2?

done

mbien

I do not recall details of what this Context is (it has no Javadoc).

the lack of comments is something I also sometimes struggle with. But most of those output listeners are little log parsing state machines. So this Context seems to keep track of the project being currently built.

This all falls apart once something enables parallelism of any kind or the log patterns change.

java/maven.junit/src/org/netbeans/modules/maven/junit/JUnitOutputListenerProvider.java

mbien · 2025-04-18T16:57:23Z

java/maven.junit/src/org/netbeans/modules/maven/junit/JUnitOutputListenerProvider.java

+                OutputVisitor.Context context = visitor.getContext();
+                if (context != null) {
+                    Project currentProject = context.getCurrentProject();
+                    if (currentProject != null) {


nitpick: you could cover both with one if chained with && like in the other comment. Those are pure field accessors with no extra code so it is ok to call them twice.

netbeans/java/maven/src/org/netbeans/modules/maven/api/output/OutputVisitor.java

Line 43 in 98e53cd

private Context context;

is not final so the current idiom seems safer in the face of potential concurrent modifications.

mbien · 2025-04-18T17:12:26Z

just as reminder: don't merge this even when approved since merges to delivery are done by the release manager.

jglick · 2025-04-21T13:25:22Z

This all falls apart once something enables parallelism of any kind

Note that there is no parallelism involved in this test case.

…mment)

jglick · 2025-04-21T13:39:24Z

java/maven.junit/src/org/netbeans/modules/maven/junit/JUnitOutputListenerProvider.java

+            Project cp = context.getCurrentProject();
+            if (cp != null) {
+                NbMavenProject subProject = cp.getLookup().lookup(NbMavenProject.class);
+                if (subProject != null) {


(Ignore whitespace changes to review.)

mbien

cool, thanks! feel free to squash as usual.

mbien · 2025-04-21T14:47:19Z

This all falls apart once something enables parallelism of any kind

Note that there is no parallelism involved in this test case.

yes I know. It was a general comment that the whole system is very fragile and many features have to turn themself off if mvnd or -T is detected.

jglick · 2025-04-21T15:35:06Z

squash as usual

Sorry, I have no idea what the usual workflow in this repo is these days. Best if an active maintainer merges PRs. I think I have write access for historical reasons, but triage would be more appropriate.

neilcsmith-net · 2025-04-21T15:46:21Z

@jglick squash and force push to your own branch.

ebarboni · 2025-04-24T09:04:27Z

Merging

mbien · 2025-05-28T01:30:36Z

the NPE turned out to be a symptom of a larger regression, #8539 is a followup.

jglick added the Maven [ci] enable "build tools" tests label Apr 18, 2025

jglick requested review from lkishalmi and mbien April 18, 2025 00:01

lkishalmi previously requested changes Apr 18, 2025

View reviewed changes

jglick force-pushed the JUnitOutputListenerProvider branch from eb99157 to 98e53cd Compare April 18, 2025 11:46

jglick changed the base branch from master to delivery April 18, 2025 11:46

mbien added this to the NB26 milestone Apr 18, 2025

jglick requested a review from lkishalmi April 18, 2025 11:47

mbien reviewed Apr 18, 2025

View reviewed changes

jglick added a commit to jglick/netbeans that referenced this pull request Apr 21, 2025

Using nested if blocks rather than ternary operator apache#8433 (co…

e501353

…mment)

jglick commented Apr 21, 2025

View reviewed changes

mbien approved these changes Apr 21, 2025

View reviewed changes

NPEs in JUnitOutputListenerProvider

08d606f

jglick force-pushed the JUnitOutputListenerProvider branch from e501353 to 08d606f Compare April 21, 2025 16:01

ebarboni merged commit acc6180 into apache:delivery Apr 24, 2025
33 checks passed

jglick deleted the JUnitOutputListenerProvider branch April 24, 2025 20:53

mbien mentioned this pull request May 28, 2025

Fix mvnd test session regression #8539

Merged

mbien added the Regression This used to work! label May 28, 2025

NPEs in JUnitOutputListenerProvider #8433

NPEs in JUnitOutputListenerProvider #8433

Uh oh!

Conversation

jglick commented Apr 18, 2025 • edited by mbien Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR approval and merge checklist:

Uh oh!

lkishalmi left a comment

Choose a reason for hiding this comment

Uh oh!

mbien left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mbien Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

jglick Apr 21, 2025

Choose a reason for hiding this comment

Uh oh!

mbien commented Apr 18, 2025

Uh oh!

jglick commented Apr 21, 2025

Uh oh!

jglick Apr 21, 2025

Choose a reason for hiding this comment

Uh oh!

mbien left a comment

Choose a reason for hiding this comment

Uh oh!

mbien commented Apr 21, 2025

Uh oh!

jglick commented Apr 21, 2025

Uh oh!

neilcsmith-net commented Apr 21, 2025

Uh oh!

ebarboni commented Apr 24, 2025

Uh oh!

Uh oh!

mbien commented May 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

NPEs in `JUnitOutputListenerProvider` #8433

NPEs in `JUnitOutputListenerProvider` #8433

jglick commented Apr 18, 2025 •

edited by mbien

Loading