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

C77 on 0.68.x test failure #5954

Closed
bsclifton opened this issue Sep 9, 2019 · 2 comments
Closed

C77 on 0.68.x test failure #5954

bsclifton opened this issue Sep 9, 2019 · 2 comments

Comments

@bsclifton
Copy link
Member

Description

When running browser tests, there are two failures

Steps to Reproduce

  1. Build Upgrade from Chromium 76.0.3809.132 to Chromium 77.0.3865.75 (uplift to 0.68.x) #5943
  2. Run npm run test brave_browser_tests -- Release
  3. See two failures

Actual result:

2 tests failed:
    BraveClearDataOnExitTwoBrowsersTest.OneGuest (../../brave/browser/browsing_data/brave_clear_browsing_data_browsertest.cc:335)
    BraveClearDataOnExitTwoBrowsersTest.OneGuestExitsLast (../../brave/browser/browsing_data/brave_clear_browsing_data_browsertest.cc:352)

Expected result:

No failures

Reproduces how often:

100%

@bsclifton bsclifton added this to the 0.68.x - Release Hotfix 2 milestone Sep 9, 2019
@bsclifton bsclifton changed the title C77 on 0.68.x C77 on 0.68.x test failure Sep 9, 2019
@petemill
Copy link
Member

petemill commented Sep 9, 2019

As @mkarolin found, pulling in brave/brave-core#2957 fixes this, which was done on 0.69.x with c77 and fixed exactly the same test issues. What 2957 does is removes calls to SetWebUIProperty.

However, 0.68.x NTP is too far away from 0.70.x to easily merge 2957 (which is on 0.70).

0.69 (can easily get 2957 cherry-picked to)

NTP: Added preferences and toggles for Clock / Stats / TopSites
NTP: Move from brave-ui to brave-core
NTP: Make menu keyboard accessible

0.68

NTP: Added preference toggle for background image

Our options are

  1. disable those tests in 68 and hope we don't hit the error in real usage
    • Pro: extremely quick
    • Con: Not great to disable tests, and we still don't know why it's an issue. However, it does seem to only be affecting Guest (and possibly Tor) windows.
  2. pull in 2957 to 68 and whatever else it needs (most of the 69 list)
    • Pro: fixes the issue
    • Con: bring in more untested features, although a known set affecting the NTP
  3. Create a new version of 2957 against 68 that uses current 68 code
    i.e. migrate 0.68 to not use SetWebUIProperty
    • Pro: only changes backend code, not bringing in new features
    • Con: May take a few hours to complete

@bsclifton
Copy link
Member Author

Fixed with #5954

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants