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 timing issue with ProcessPendingContributions browser test #5498

Closed

Conversation

emerick
Copy link
Contributor

@emerick emerick commented May 8, 2020

Resolves brave/brave-browser#9687

Submitter Checklist:

Test Plan:

Run the following command and verify that the test passes:

brave_browser_tests --gtest_filter=BraveRewardsBrowserTest.ProcessPendingContributions

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@emerick emerick requested a review from NejcZdovc as a code owner May 8, 2020 20:32
@emerick emerick self-assigned this May 8, 2020
@emerick emerick force-pushed the rewards-test-process-pending-contributions-fix branch from 54318e6 to 3ac72b2 Compare May 8, 2020 22:10
rewards_service_browsertest_utils::WaitForElementThenClick(
OpenRewardsPopup(),
"[data-test-id='unverified-check-button']");
// Refresh publisher list; assume that absence of refresh button
Copy link
Contributor

Choose a reason for hiding this comment

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

how come publisher is already verified if we didn't click refresh yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding of the code the publisher list also updates on a timer, so sometimes the manual click isn't needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

interesting so you thing that timer is causing problems

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the timer is kicking off the update and updating the server before there's any chance to click the button (at least on my local Windows machine). Once the rewards popup opens, there's no longer a "Refresh Status" button to click on because it's already updated to a verified publisher.

Copy link
Contributor

Choose a reason for hiding this comment

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

this happened because we don't send any end when doing pagination, so pagination will continue for 100 pages and because of that we have this race condition

@emerick emerick force-pushed the rewards-test-process-pending-contributions-fix branch from 3ac72b2 to eee5fc3 Compare May 9, 2020 14:03
@emerick
Copy link
Contributor Author

emerick commented May 10, 2020

@NejcZdovc found the real fix here #5502, closing.

@emerick emerick closed this May 10, 2020
@NejcZdovc NejcZdovc deleted the rewards-test-process-pending-contributions-fix branch May 10, 2020 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BraveRewardsBrowserTest.ProcessPendingContributions failing on linux
2 participants