Get Chromium browser_tests working #8297
Comments
|
Also for the record, I see a lot of test failures already when running this locally. Stuff that likely could be causing issues. I have no idea what runs in CI however it's must not be running at least for a debug build. |
|
@jonathanKingston do you have more information about this? ex: what error are you getting? I wasn't aware that we're exposing a way to build the unit / browser tests for Chromium. Will take a look at this |
|
OK reproduced: Going to pull your branch down and give it a go 😄 |
|
@bsclifton it's noisy certainly, however the I'm hoping to chase down the following error lines:
Which account for well over 1k of the console lines. |
|
@jonathanKingston I think you'll need to modify brave-browser/scripts/commands.js Lines 159 to 170 in a3299c7 and also: Line 40 in a3299c7 Although, I think it's working as expected (since it passes the Definitely missing something- maybe the Chromium browser tests would need to be patched to include test files from Ads / Ledger (to register a stub for the service). But I did verify your patch gets it compiling 😄 |
Yeah I agree, however it seems to be test specific. The error isn't in every every test and the browser appears to have the code. |
|
Similar to what I mentioned in detail in #8376 (comment) for the case of getting And the good news is that I just got them building with a very small adjustment in the source overrides for //c/b/renderer_context_menu/render_view_context_menu.h: And since it's just such a small change, I've repurposed #14001 so that it's not just about fixing the build for I'm going to look next into which browser_tests are failing and which ones we could easily skip in a similar fashion to how I did it for unit_tests in brave/brave-core#7901, but for the time being I thought a heads up was in order. PTAL to brave/brave-core#7899 when you have a moment, thanks! |
This patch excludes entire *_browsertest.cc files from being compiled
so that we avoid running any test that we know are going to fail due
to the many ways in which Brave is different than Chromium upstream.
Note that it's possible that this way of excluding tests is a bit too
agressive (i.e. it's likely that some browser tests inside the excluded
files would pass), but for now it's a good enough initial approach as
it enables us to considerably increase test coverage without having
to maintain patches in a too intrusive way.
As a reference, a this time of this patch's writing (on top of Brave
1.22.18 / Chromium 89.0.4389.40), these are the stats when running
upstream's browser tests on a Linux/Debug build, without this patch:
* Total run: 13337 tests
* Passed: 11429 tests
* Not passed: 1908 tests
- Failed: 871 tests
- Crashed: 555 tests
- Timed out: 482 tests
With this patch applied, the numbers look like this:
* Total run: 6084 tests
* Passed: 6084 tests
* Not passed: 0 tests
That is, what we have with this patch applied looks as follows:
* 6084/13337 -> 45.62% of ALL the original tests being run
* 6084/11429 -> 53.23% of the PASSING TESTS still being run
In other words, we're increasing test coverage in 6084 browser tests
in a relatively clean way (i.e. no complex patching) while losing
46.77% of the browser tests that would run and pass if we were not
excluding them in such an agressive way. Not as good results as for
the case of unit tests, but still an improvement for test coverage.
Fix brave/brave-browser#8297
This patch disables entire *_browsertest.cc files by making them empty
via chromium_src overrides, as well as all the JS tests via a patch to
include_js_tests.gni, so that we avoid running any test that we know
are going to fail because of the many ways in which Brave is different
than Chromium upstream.
Note that it's possible that this way of excluding tests is a bit too
agressive (i.e. it's likely that some browser tests inside the excluded
files would pass), but for now it's a good enough initial approach as
it enables us to considerably increase test coverage without having
to maintain patches in a too intrusive way.
As a reference, a this time of this patch's writing (on top of Brave
1.23.30 / Chromium 89.0.4389.86), this are the stats when running
upstream's unit tests on a Linux/Debug build, without this patch:
* Total run: 13355 tests
* Passed: 10427 tests
* Not passed: 2928 tests
- Failed: 870 tests
- Crashed: 1593 tests
- Timed out: 465 tests
With this patch applied, the numbers look like this:
* Total run: 2957 tests
* Passed: 2957 tests
* Not passed: 0 tests
That is, what we have with this patch applied looks as follows:
* 2957/13355 -> 22.14% of ALL the original tests being run
* 2957/10427 -> 28.36% of the PASSING TESTS still being run
In other words, we're increasing test coverage in 2957 browser tests
in a relatively clean way (i.e. no complex patching) while losing
71.64% of the browser tests that would run and pass if we were not
excluding them in such an agressive way. Not as good results as for
the case of unit tests, but still an improvement for test coverage.
Fix brave/brave-browser#8297
This patch disables entire *_browsertest.cc files by making them empty
via chromium_src overrides, as well as all the JS tests via a patch to
include_js_tests.gni, so that we avoid running any test that we know
are going to fail because of the many ways in which Brave is different
than Chromium upstream.
Note that it's possible that this way of excluding tests is a bit too
agressive (i.e. it's likely that some browser tests inside the excluded
files would pass), but for now it's a good enough initial approach as
it enables us to considerably increase test coverage without having
to maintain patches in a too intrusive way.
As a reference, a this time of this patch's writing (on top of Brave
1.23.30 / Chromium 89.0.4389.86), this are the stats when running
upstream's unit tests on a Linux/Debug build, without this patch:
* Total run: 13355 tests
* Passed: 10427 tests
* Not passed: 2928 tests
- Failed: 870 tests
- Crashed: 1593 tests
- Timed out: 465 tests
With this patch applied, the numbers look like this:
* Total run: 2957 tests
* Passed: 2957 tests
* Not passed: 0 tests
That is, what we have with this patch applied looks as follows:
* 2957/13355 -> 22.14% of ALL the original tests being run
* 2957/10427 -> 28.36% of the PASSING TESTS still being run
In other words, we're increasing test coverage in 2957 browser tests
in a relatively clean way (i.e. no complex patching) while losing
71.64% of the browser tests that would run and pass if we were not
excluding them in such an agressive way. Not as good results as for
the case of unit tests, but still an improvement for test coverage.
Fix brave/brave-browser#8297
Running
npm run test -- browser_testscurrently ends in a compiler error preventing any of the tests to run.This means I can't verify Chromium test are broken with other changes such as: #8184
The text was updated successfully, but these errors were encountered: