Avoid NullPointerException when deserializing TestIdentifier

[dmlloyd]

Overview

Fix NullPointerException when a TestIdentifier with no parentId is deserialized.

Issue: #3819

---

I hereby agree to the terms of the JUnit Contributor License Agreement.

---

Definition of Done

• There are no TODOs left in the code
• Method preconditions are checked and documented in the method's Javadoc
• Coding conventions (e.g. for logging) have been followed
• Change is covered by automated tests including corner cases, errors, and exception handling
• Public API has Javadoc and @API annotations
• Change is documented in the User Guide and Release Notes

[marcphilipp]

Thanks for reporting and providing a fix! Could you please add a test to org.junit.platform.launcher.TestIdentifierTests to cover this scenario? It should suffice to serialize and deserialize TestIdentifier.from(engineDescriptor).

[dmlloyd]

I've pushed up a test (and good thing too, because it uncovered a second place where the problem occurs). I don't really understand how the descriptors and identifiers work together though so I just created an anonymous instance of TestDescriptors with the exact characteristics that trigger the problem. Is that OK?

[dmlloyd]

Hmm CI is failing due to a spotless check for a file I didn't touch: https://github.com/junit-team/junit5/actions/runs/9114769155/job/25059506872?pr=3820#step:3:731

Did I mess something up somehow?

To quote the great man: "I wasn't even in aisle seven!"

[dmlloyd]

Also one more question... is there any chance that this change could be backported into a 5.10.3 release?

[marcphilipp]

I don't really understand how the descriptors and identifiers work together though so I just created an anonymous instance of TestDescriptors with the exact characteristics that trigger the problem. Is that OK?

I polished the test to make it more like the others in that class in 85a300c.

Hmm CI is failing due to a spotless check for a file I didn't touch: junit-team/junit5/actions/runs/9114769155/job/25059506872?pr=3820#step:3:731

Did I mess something up somehow?

That was my bad. Earlier today, I pushed something to main that broke that. I just fixed it and merged the fix into your branch. Should be good now.

Also one more question... is there any chance that this change could be backported into a 5.10.3 release?

Would a 5.11.0-M2 release also work for you?

[dmlloyd]

I don't really understand how the descriptors and identifiers work together though so I just created an anonymous instance of TestDescriptors with the exact characteristics that trigger the problem. Is that OK?

I polished the test to make it more like the others in that class in 85a300c.

Great! Thanks.

Hmm CI is failing due to a spotless check for a file I didn't touch: junit-team/junit5/actions/runs/9114769155/job/25059506872?pr=3820#step:3:731
Did I mess something up somehow?

That was my bad. Earlier today, I pushed something to main that broke that. I just fixed it and merged the fix into your branch. Should be good now.

Awesome!

Also one more question... is there any chance that this change could be backported into a 5.10.3 release?

Would a 5.11.0-M2 release also work for you?

I'm not sure; I'll follow up once I've consulted with the team.

[geoand]

Would a 5.11.0-M2 release also work for you?

Is there any chance we can see a preliminary changelog for 5.11?
We have never encountered any issues upgrading in the past, but I would nevertheless like us to do our due diligence :)

[marcphilipp]

Sure, here are the release notes for M1 and the preliminary ones for M2.

[geoand]

Thanks!

[marcphilipp]

@dmlloyd @geoand We just released 5.11.0-M2

[geoand]

🙏

[geoand]

@marcphilipp thanks again, for the release!
We tried and it and this fix works expected, however because the milestone release has some other effects that we would ideally like to avoid dealing with until a GA, release, is there any chance we could get a 5.10.3?

Thanks a lot

[dmlloyd]

And relatedly, would it be useful if I put up a backport of this PR to the 5.10.x branch?

[dmlloyd]

Just in case, I've opened #3837.

[sbrannen]

@dmlloyd and @geoand,

I've opened #3838 so the team can consider backporting selected bug fixes to 5.10.x.

[geoand]

Thanks a lot @sbrannen!