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

Tests: Bump pruning size value for unit test #6087

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

brbrr
Copy link
Contributor

@brbrr brbrr commented Sep 11, 2023

Update_known_chain_sizes is failing on master, which blocks PR merging to master.

Changes

  • Increase boundary prunning size value for the Update_known_chain_sizes test

Types of changes

What types of changes does your code introduce?

  • (Temporary) Fix for a unit test

Testing

Requires testing

  • Yes
  • No

@brbrr brbrr self-assigned this Sep 11, 2023
Copy link
Member

@tkstanczak tkstanczak left a comment

Choose a reason for hiding this comment

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

Add test. What would be a good test? Maybe it should fail if there was not update to this file for more than a month?

@brbrr
Copy link
Contributor Author

brbrr commented Sep 11, 2023

From a QA standpoint, the test is fragile by design since it relies on data outside of the test controls. Thus, the behavior of the test cannot be deterministic. Having a test that checks the file updates might work if the pruning size is growing linearly, which I don't know.

Ideally, we'd fetch the expected values on every test run, but it may defeat the purpose of the test :)

Anyway, currently tests on the master are failing, which means all the PRs are blocked. Maybe we can merge it as-is, and then create a follow-up issue/PR with a long-term solution?

Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

All this are estimates on moving targets, not sure how would you test it

@LukaszRozmej LukaszRozmej merged commit 608f6cf into master Sep 11, 2023
61 checks passed
@LukaszRozmej LukaszRozmej deleted the fix/test-bump-mainnet-size branch September 11, 2023 17:22
@tkstanczak
Copy link
Member

@LukaszRozmej you can write a test that checks with github hidden folder to verify when the file got last time updated. And fail if it was not updated recently enough.

@tkstanczak
Copy link
Member

Oh, this is a test file already? Sorry, I thought ir was some production code.

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.

None yet

7 participants