-
Notifications
You must be signed in to change notification settings - Fork 920
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 android test overrides #20825
Fix android test overrides #20825
Conversation
} | ||
|
||
#if BUILDFLAG(IS_WIN) | ||
// Enable high-DPI for interactive tests where the user is expected to | ||
// Many tests validate code that requires user32.dll to be loaded. Loading it, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the latest changes to chrome_test_launcher I think this file can be eliminated and replaced with upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes here are pulled in from upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
++
#endif // BUILDFLAG(IS_WIN) | ||
} | ||
|
||
ChromeTestSuiteRunner runner; | ||
BraveTestLauncherDelegate delegate(&runner); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you #define
-override ChromeTestLauncherDelegate
with BraveTestLauncherDelegate
and just use browser_tests_runner
from chrome?
both chromium and brave tests can use BraveTestLauncherDelegate
, that should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes as mentioned here https://github.com/brave/brave-core/pull/20825/files#r1382007273 but I was leaving this for a follow-up because I want to get what I have here merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cause of failure for these tests was fixed in #20825.
The cause of failure for these tests was fixed in #20825.
The cause of failure for these tests was fixed in #20825.
The cause of failure for these tests was fixed in #20825.
Resolves brave/brave-browser#27837
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: