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

fix(a3p-integration): replaceFeeDistributor flakiness fix #11006

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

Conversation

anilhelvaci
Copy link
Collaborator

closes: #10998

Description

In #10998, we see a flake introduced after a3p-integration/p:upgrade-19 tests migrated to a3p-integration/n:upgrade-next. The investigations showed that feeDistributor was collecting more fees than our test is producing.

Passing

2025-02-11T22:51:00.145Z SwingSet: vat: v36: ----- ReserveKit.5  9 received collateral { brand: Object [Alleged: IST brand] {}, value: 500_000n }

Failing

2025-02-11T22:04:40.861Z SwingSet: vat: v36: ----- ReserveKit.5  8 received collateral { brand: Object [Alleged: IST brand] {}, value: 550_000n }

My argument is, priorly we were introducing the new feeDistributor dedicated to testing during our test initiation phase. And after introduction of the feeDistributor, we were waiting for one full collectionInterval to clear any leftover fees that haven't been collected by the old feeDistributor. And only then we start producing new fees. I confirmed that this is not the current behavior after the n:upgrade-next migration as of the commit 271c55f#diff-7dd0ed44de879823d10b252f6483f52174e4fb9240966e7fcd1754298b41b0d7L161

  await evalBundles(coreEvalDir);
  // Wait for a round to give the new feeDistributor time to clear out outstanding fees, if any.
  await sleep(collectionInterval + 5000, { log: console.log, setTimeout });

As of the migration to upgrade-19, we introduce the new feeDistributor in uprage.go which causes fees produced by other tests to get in the way of replaceFeeDistributor.test.js in a flakey way since the collectionInterval is 30 seconds.

Solution

Just switch from AmountMath.isEqual to AmountMath.isGTE when polling for results.

Testing Considerations

Make sure integration tests pass in a consistent way.

Upgrade Considerations

Get rid of the flake so upgrade-19 shipment isn't blocked.

@anilhelvaci anilhelvaci added the force:integration Force integration tests to run on PR label Feb 14, 2025
@anilhelvaci anilhelvaci self-assigned this Feb 14, 2025
@anilhelvaci anilhelvaci requested a review from turadg February 14, 2025 19:26
@anilhelvaci
Copy link
Collaborator Author

CI fails in multichain tests:

I haven't been able to find anything related to feeDistributor in the logs. You mentioned multichain test failures yesterday as well. Are these the same as the ones you saw? @turadg

@anilhelvaci anilhelvaci marked this pull request as ready for review February 14, 2025 19:26
@anilhelvaci anilhelvaci requested a review from a team as a code owner February 14, 2025 19:26
@turadg
Copy link
Member

turadg commented Feb 14, 2025

CI fails in multichain tests:

Independent flake,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replaceFeeDistributor flake
2 participants