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

Refactor: log4j-api-test to junit 5 #9

Open
wants to merge 3 commits into
base: 2.x
Choose a base branch
from

Conversation

Realtin
Copy link

@Realtin Realtin commented Oct 23, 2024

[A clear and concise description of what the pull request is for along with a reference to the associated issue IDs, if they exist.]

Checklist

  • Base your changes on 2.x branch if you are targeting Log4j 2; use main otherwise
  • ./mvnw verify succeeds (if it fails due to code formatting issues reported by Spotless, simply run ./mvnw spotless:apply and retry)
  • Non-trivial changes contain an entry file in the src/changelog/.2.x.x directory
  • Tests for the changes are provided
  • Commits are signed (optional, but highly recommended)

Copy link

github-actions bot commented Oct 23, 2024

Job Requested goals Build Tool Version Build Outcome Build Scan®
build-macos-latest clean install 3.9.8 Build Scan NOT_PUBLISHED
build-ubuntu-latest clean install 3.9.8 Build Scan NOT_PUBLISHED
build-windows-latest clean install 3.9.8 Build Scan NOT_PUBLISHED
Generated by gradle/develocity-actions

@Realtin Realtin force-pushed the log4j-api-test_refactor_junit5 branch 2 times, most recently from 9d64a78 to 4d0c11a Compare October 28, 2024 17:10
@Realtin Realtin changed the title log4j-api-test WIP Refactor: log4j-api-test to junit 5 Oct 29, 2024
@@ -123,6 +123,7 @@ public void testSerializationOfNonSerializableValue() throws Exception {
public void testDeserializationOfUnknownClass() throws Exception {
final SortedArrayStringMap original = new SortedArrayStringMap();
original.putValue("a", "avalue");
// org.junit.runner does not have an equivalent in junit 5
original.putValue("serializableButNotInClasspathOfDeserializer", new org.junit.runner.Result());
Copy link
Author

Choose a reason for hiding this comment

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

This is a part I just don't know how to fix. It seems to me as if they are just using the Result() function to generate a Serializable output. Maybe they need to rewrite this test.

@Realtin
Copy link
Author

Realtin commented Oct 29, 2024

All of a sudden I am now getting the

org.apache.maven.surefire.booter.SurefireBooterForkException: ExecutionException Error occurred in starting fork

Error locally when running the tests.

Which seems to be cause by a loop maybe here:

org.apache.logging.log4j.util.LoaderUtilSecurityManagerTest$TestSecurityManager.checkPermission(LoaderUtilSecurityManagerTest.java:36)

this used to only happen when I removed the junit dependency from the pom.xml file and replaced it with org.junit.vinatge.

I talked to james about this error the other day and we did not have an idea how to figure out what is causing this. so we left it (since at the time I thought it only occurs with the junit.vinatge dependency which I could just not use)

The other Build Actions will also fail (probably due to some timeout or Stackoverflow somewhere) :(((

@Realtin Realtin force-pushed the log4j-api-test_refactor_junit5 branch from 0760ff0 to 2b9940a Compare October 29, 2024 12:11

@ParameterizedTest
@MethodSource("data")
public void testSerializationRoundtripEquals(Serializable serializable) {
Copy link

Choose a reason for hiding this comment

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

oopsie, this should be final MessageFormatMessage message

@Realtin Realtin force-pushed the log4j-api-test_refactor_junit5 branch from 2b9940a to 4ca67e8 Compare November 19, 2024 10:49
@Realtin Realtin force-pushed the log4j-api-test_refactor_junit5 branch from 4ca67e8 to b15b1e9 Compare November 19, 2024 12:15
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