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

test: remove redundant contractGovernor test #10907

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

turadg
Copy link
Member

@turadg turadg commented Jan 28, 2025

refs: #10906

Description

What swingsetTests/contractGovernor was written to cover is now thoroughly covered in bootstrap and a3p tests.

Moreover it takes two minutes on a MBP and it's a style of test we want to avoid going forward.

So this removes a suite that has costs and no added value.

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

no loss in coverage

Upgrade Considerations

n/a

It's thoroughly covered in bootstrap and a3p tests now.

This takes over two minutes on my MacBook Pro without adding any coverage. Moreover it's a style of test we want to avoid going forward.
@turadg turadg requested a review from Chris-Hibbert January 28, 2025 18:50
@turadg turadg requested a review from a team as a code owner January 28, 2025 18:50
@turadg
Copy link
Member Author

turadg commented Jan 28, 2025

This needs more work to refactor what was depending on these files. I'm leaving R4R to get feedback on the removal per se.

Copy link
Contributor

@Chris-Hibbert Chris-Hibbert left a comment

Choose a reason for hiding this comment

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

I don't think that we have other tests that check what oneVoterValidate() verifies. I don't know of other tests of offerFilters. (look for voteonOfferFilter.)

There are plenty of tests of API invocation and changing governed params, as well as updating electorates.

@turadg turadg marked this pull request as draft January 29, 2025 01:49
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