Skip to content

Migrate Transactions, Partitions and Statistics files in Core to JUnit5#9999

Merged
nastra merged 4 commits intoapache:mainfrom
tomtongue:mig-junit5-core-tx-part-stats
Mar 20, 2024
Merged

Migrate Transactions, Partitions and Statistics files in Core to JUnit5#9999
nastra merged 4 commits intoapache:mainfrom
tomtongue:mig-junit5-core-tx-part-stats

Conversation

@tomtongue
Copy link
Contributor

@tomtongue tomtongue commented Mar 19, 2024

Migrate the following test classes in iceberg-core to JUnit 5 and AssertJ style for #9085.

Current Progress

Transactions

  • TestCreateTransaction.java
  • TestReplaceTransaction.java
  • TestTransaction.java

Partitions:

  • TestPartitioning.java
  • TestPartitionSpecInfo.java
  • TestPartitionSpecParser.java

Statistics

  • TestSetPartitionStatistics.java
  • TestSetStatistics.java

Other

  • TestCommitReporting.java

@github-actions github-actions bot added the core label Mar 19, 2024
@tomtongue tomtongue changed the title [WIP] Migrate Transactions, Partitions and Statistics files in Core to JUnit5 Migrate Transactions, Partitions and Statistics files in Core to JUnit5 Mar 19, 2024
assertThat(spec.isUnpartitioned()).isTrue();
assertThat(table.spec()).isEqualTo(spec);
assertThat(table.spec().lastAssignedFieldId()).isEqualTo(spec.lastAssignedFieldId());
assertThat(table.specs()).containsEntry(spec.specId(), spec);
Copy link
Contributor

Choose a reason for hiding this comment

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

this should do isEqualTo() as in the previous version of the code

Copy link
Contributor

Choose a reason for hiding this comment

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

or alternatively you can also use containsExactly(...)

assertThat(table.spec()).isEqualTo(spec);
assertThat(table.spec().lastAssignedFieldId()).isEqualTo(spec.lastAssignedFieldId());
assertThat(table.specs()).containsEntry(spec.specId(), spec);
assertThat(table.specs().get(Integer.MAX_VALUE)).isNull();
Copy link
Contributor

Choose a reason for hiding this comment

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

doesNotContainEntry()

assertThat(table.spec()).isEqualTo(spec);
assertThat(table.spec().lastAssignedFieldId()).isEqualTo(spec.lastAssignedFieldId());
assertThat(table.specs()).containsEntry(spec.specId(), spec);
assertThat(table.specs().get(Integer.MAX_VALUE)).isNull();
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above for both

Copy link
Contributor

Choose a reason for hiding this comment

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

please also update all other places

@tomtongue
Copy link
Contributor Author

@nastra Thanks for the review. Could you check the new one when you have time?

@nastra nastra merged commit fae0f81 into apache:main Mar 20, 2024
@tomtongue
Copy link
Contributor Author

@nastra Thank you! If possible, could you review another PR I previously created; #9994

@tomtongue tomtongue deleted the mig-junit5-core-tx-part-stats branch March 20, 2024 07:15
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
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