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

[SUREFIRE-1914] Draft: Document expected XML report format for @ParameterizedTest with ITs #351

Closed
wants to merge 1 commit into from

Conversation

andpab
Copy link
Contributor

@andpab andpab commented May 16, 2021

This is just a draft to document what my expectations - as also described in the issue - would be. Obviously that means some tests will fail.

This draft is meant to form a basis for discussion, not to be merged directly.

+ "classname=\"junitplatformenginejupiter.BasicJupiterTest\"" )
.assertContainsText( "<testcase name=\"1 + 2 = 3\" "
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the old method? I cannot see it here in this PR.
I would like to keep it and add new ones because then we have more alternatives. Would it be possible?
Pls check our documentation because sometimes we use the reference pointing to our IT tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will rewrite the PR, so that it does not change existing ITs. Thanks for your feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rewrote it, so that only a new dedicated test project and IT for it is added.

Test project: surefire-1914-xml-reporting-parameterizedtest
IT: Surefire1914XmlReportingParameterizedTestIT.java

Please have a look again.

For reference: The relevant pre-existing IT with a divergent expectation is JUnitPlatformEnginesIT#testJupiterEngineWithDisplayNames

</plugins>
</build>

<repositories>
Copy link
Contributor

@Tibor17 Tibor17 May 22, 2021

Choose a reason for hiding this comment

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

Pls remove the repository. Our build uses the artifacts from your local repo. It means the integration tests expect mvn install which packages and installs the project artifacts in your local repo on your disk. Pls run

mvn install -DskipTests
mvn verify -rf :surefire-its -nsu -P run-its -Dit.test=Surefire1914XmlReportingParameterizedTestIT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the repository definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, I am trying to build your project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Tibor17 I noticed there were still a couple of inconsistencies in my expectations. I force-pushed some changes. Please checkout anew.

@Tibor17
Copy link
Contributor

Tibor17 commented May 24, 2021

@andpab
Do you want to continue with the RunListenerAdapter? I may guide you and then the tests will pass successfully.

@andpab
Copy link
Contributor Author

andpab commented May 26, 2021

Ok, I've given it a shot. Opened a second draft PR: #352

Please give me some feedback.

@Tibor17 Tibor17 closed this Jul 13, 2021
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

Successfully merging this pull request may close these issues.

2 participants