Skip to content

Conversation

@tomtongue
Copy link
Contributor

@tomtongue tomtongue commented Mar 19, 2025

Migrate Spark 3.4 tests based on JUnit 4 to Junit5 with AssertJ style. This is related to #7160

This PR migrates the other tests in the spark.actions package in Spark 3.4 into JUnit 5 with AssertJ. The following classes are included in this PR:

@github-actions github-actions bot added the spark label Mar 19, 2025
@tomtongue
Copy link
Contributor Author

tomtongue commented Mar 19, 2025

Maybe the changes for TestRewriteTablePathsAction seem to conflict with #12568. Let me remove the changes for that test in this commit.

@tomtongue tomtongue force-pushed the spark3.4-testbase-remaining-actions branch from 3962c5d to 665adf9 Compare March 19, 2025 16:47
@tomtongue
Copy link
Contributor Author

@nastra Could you review this PR when you have time?


assertThat(result.rewriteResults()).as("Should have 1 fileGroups").hasSize(1);
assertThat(result.rewrittenBytesCount()).isEqualTo(dataSizeBefore);
assertThat(result.rewriteResults()).as("Should have 1 fileGroups").hasSize(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was mistakenly removed and should be added back

StreamSupport.stream(result.orphanFileLocations().spliterator(), false)
.anyMatch(file -> file.contains("v1.metadata.json")));
assertThat(result.orphanFileLocations()).as("Should delete 1 file").hasSize(1);
assertThat(StreamSupport.stream(result.orphanFileLocations().spliterator(), false))
Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean to backport the change here without using StreamSupport?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, thanks so much for pointing this out. Let me fix.

Copy link
Contributor

@nastra nastra left a comment

Choose a reason for hiding this comment

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

thanks @tomtongue

@tomtongue
Copy link
Contributor Author

Thanks for the review!! @nastra

@nastra nastra merged commit 2c157cb into apache:main Mar 21, 2025
27 checks passed
lliangyu-lin pushed a commit to lliangyu-lin/iceberg that referenced this pull request Mar 23, 2025
@tomtongue tomtongue deleted the spark3.4-testbase-remaining-actions branch September 17, 2025 05:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants