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

Core: Cleanup unit tests #12666

Merged
merged 1 commit into from
Mar 28, 2025
Merged

Conversation

sullis
Copy link
Contributor

@sullis sullis commented Mar 27, 2025

Motivation

Some unit tests declare @ Parameters containing a format version list.

The parent class, TestBase, already provides @ Parameters with
the format version list.

@github-actions github-actions bot added the core label Mar 27, 2025
@sullis sullis force-pushed the sullis/cleanup-TestFindFiles branch from 46f9d33 to f5dbf09 Compare March 27, 2025 15:11
@ebyhr
Copy link
Contributor

ebyhr commented Mar 28, 2025

TestFindFiles isn't the only class having 1, 2, 3 formatVersion. For instance, TestCreateTransaction and TestSnapshot also have the same parameter.

@nastra
Copy link
Contributor

nastra commented Mar 28, 2025

There are more subclasses of TestBase that define the same parameters as the base class, so it would be good to clean those up as well

@sullis sullis force-pushed the sullis/cleanup-TestFindFiles branch from f5dbf09 to bdb8a41 Compare March 28, 2025 16:22
@sullis sullis changed the title Core: Cleanup TestFindFiles Core: Cleanup unit tests Mar 28, 2025
@sullis
Copy link
Contributor Author

sullis commented Mar 28, 2025

I pushed an update to this branch. I identified 34 unit tests that needed cleanup.

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.

LGTM, thanks for cleaning this up @sullis

@nastra nastra merged commit 9199ab5 into apache:main Mar 28, 2025
42 checks passed
@sullis sullis deleted the sullis/cleanup-TestFindFiles branch March 28, 2025 18:57
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.

3 participants