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

testfix: flaky tests fix attempt #410

Merged
merged 2 commits into from
Jul 10, 2024
Merged

Conversation

mFragaBA
Copy link
Contributor

@mFragaBA mFragaBA commented Jul 1, 2024

addresses #391. (The PR is already reviewable but I think this will be included in the following release).

In some tests we do some assertions with client.get_input_notes(filter).unwrap() and checking the length of it. While in most cases that is enough, it will sporadically fail If the node running the integration test has public notes belonging to another account with the same account id prefix (the one used to create the note tag). In order to mitigate it, we do a filter by account_id to ensure the note we want is there (there might be others, we won't care too much). In the case of swap tests, I also used get_consumable_notes since in that situation we wanted to consume the notes anyways.

@mFragaBA mFragaBA force-pushed the mFragaBA-flaky-tests-fix-attempt branch from a8142e5 to 045ed8b Compare July 1, 2024 14:40
@mFragaBA mFragaBA marked this pull request as ready for review July 1, 2024 18:48
@mFragaBA mFragaBA linked an issue Jul 1, 2024 that may be closed by this pull request
@mFragaBA mFragaBA marked this pull request as draft July 3, 2024 13:28
@mFragaBA mFragaBA force-pushed the mFragaBA-flaky-tests-fix-attempt branch from ad3c613 to 617c6cd Compare July 5, 2024 19:25
@mFragaBA mFragaBA marked this pull request as ready for review July 5, 2024 19:26
@mFragaBA mFragaBA force-pushed the mFragaBA-flaky-tests-fix-attempt branch from 617c6cd to 8e21f03 Compare July 8, 2024 17:36
mFragaBA added 2 commits July 8, 2024 15:10
In some tests we did some assertions by doing `client.get_input_notes(filter).unwrap()` and checking the length of it. While in most cases that is enough, it will sporadically become an error if the node running the integration test has notes whose tag was built with another account's id that also has the same prefix (and hence the same tag as the ones created for the actually desired account). In order to mitigate it, we do a filter by account_id to ensure the note we want is there (there might be others, we won't care too much). In the case of swap tests, I also used `get_consumable_notes` since in that situation we wanted to consume the notes anyways.
@mFragaBA mFragaBA force-pushed the mFragaBA-flaky-tests-fix-attempt branch from 8e21f03 to 82b30da Compare July 8, 2024 18:10
Copy link
Collaborator

@tomyrd tomyrd left a comment

Choose a reason for hiding this comment

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

LGTM

@igamigo igamigo merged commit 1a0a6ef into next Jul 10, 2024
11 checks passed
@igamigo igamigo deleted the mFragaBA-flaky-tests-fix-attempt branch July 10, 2024 14:34
@igamigo igamigo mentioned this pull request Jul 15, 2024
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.

Fix flaky test
3 participants