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

Delta: Fix integration tests and Create DataFile by partition values instead of path #8398

Merged
merged 11 commits into from
Dec 7, 2023

Conversation

HonahX
Copy link
Contributor

@HonahX HonahX commented Aug 26, 2023

Comment on lines -99 to +90
public TestSnapshotDeltaLakeTable(
String catalogName, String implementation, Map<String, String> config) {
super(catalogName, implementation, config);
public TestSnapshotDeltaLakeTable() {
super(icebergCatalogName, SparkCatalog.class.getName(), config);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Junit5 cannot parametrize class creation. Since we only have one set of parameters in junit4 version, I think we can directly feed them into super class.

@@ -608,6 +608,7 @@ project(':iceberg-delta-lake') {
}

task integrationTest(type: Test) {
useJUnitPlatform()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current master branch skips all the integration tests when running delta-lake CI. Seems we also need useJUnitPlatform() here when defining Junit5 integration test task.

@HonahX HonahX changed the title Delta: Create DataFile by partition values rather than path and Fix integration tests Delta: Create DataFile by partition values instead of path and Fix integration tests Aug 26, 2023
@HonahX HonahX changed the title Delta: Create DataFile by partition values instead of path and Fix integration tests Delta: Fix integration tests and Create DataFile by partition values instead of path Aug 26, 2023
@HonahX
Copy link
Contributor Author

HonahX commented Aug 26, 2023

@jackye1995 Could you please review this when you have a moment. Thanks in advance!

@jackye1995
Copy link
Contributor

Most looks good to me, CI seems to fail with unrelated issue, try to retrigger

@jackye1995 jackye1995 closed this Dec 7, 2023
@jackye1995 jackye1995 reopened this Dec 7, 2023
@jackye1995
Copy link
Contributor

CI seems to be passing now after retry. Given the fact that this issue was breaking CI, I will go ahead to merge it directly. Thanks for the fix!

@jackye1995 jackye1995 merged commit b79a8ff into apache:main Dec 7, 2023
89 of 90 checks passed
lisirrx pushed a commit to lisirrx/iceberg that referenced this pull request Jan 4, 2024
devangjhabakh pushed a commit to cdouglas/iceberg that referenced this pull request Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants