extensions: Port content_scripts API tests to desktop Android, part 1 [chromium/src : main]
James Cook (Gerrit)
James Cook added 1 comment![]( "Open in Gerrit")
Zhengzheng, please take a look. The tryjobs are red due to a compile failure on tip-of-trunk, not this CL. The tests all pass locally and I'll run tryjobs before landing. Thanks!
Related details
- Zhengzheng Liu
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zhengzheng Liu (Gerrit)
Zhengzheng Liu added 3 comments![]( "Open in Gerrit")
Consider adding `static_assert(BUILDFLAG(ENABLE_EXTENSIONS_CORE));`??
#if BUILDFLAG(ENABLE_EXTENSIONS)Is it possible to merge this `#if BUILDFLAG(ENABLE_EXTENSIONS)` block to the above one? Or they are separated intentionally due to different TODOs? I have no preference, just to clarify :)
ntp_test_utils::SetUserSelectedDefaultSearchProvider(I think `ntp_test_utils` is not available on Android. Maybe consider guard this piece of code behind ENABLE_EXTENSIONS?
Related details
- James Cook
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
James Cook (Gerrit)
James Cook voted and added 4 comments![]( "Open in Gerrit")
Votes added by James Cook
| Commit-Queue | +1 |
4 comments
James CookConsider adding `static_assert(BUILDFLAG(ENABLE_EXTENSIONS_CORE));`??
Done. Whoops!
Is it possible to merge this `#if BUILDFLAG(ENABLE_EXTENSIONS)` block to the above one? Or they are separated intentionally due to different TODOs? I have no preference, just to clarify :)
I kept them separate intentionally, since they are different TODOs (and this one will be deleted in the next CL).
ntp_test_utils::SetUserSelectedDefaultSearchProvider(I think `ntp_test_utils` is not available on Android. Maybe consider guard this piece of code behind ENABLE_EXTENSIONS?
I'd prefer to do this in the next CL, when I'm enabling tests that use this class. Is that OK? We may have to skip the entire class but I'm not sure yet.
Related details
- Zhengzheng Liu
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Zhengzheng Liu (Gerrit)
Zhengzheng Liu voted and added 1 comment![]( "Open in Gerrit")
Votes added by Zhengzheng Liu
| Code-Review | +1 |
1 comment
ntp_test_utils::SetUserSelectedDefaultSearchProvider(James CookI think `ntp_test_utils` is not available on Android. Maybe consider guard this piece of code behind ENABLE_EXTENSIONS?
I'd prefer to do this in the next CL, when I'm enabling tests that use this class. Is that OK? We may have to skip the entire class but I'm not sure yet.
I am OK with it. Please go ahead as long as the CQ run passes. 😊
Related details
- James Cook
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
James Cook (Gerrit)
James Cook voted and added 1 comment![]( "Open in Gerrit")
Votes added by James Cook
| Commit-Queue | +2 |
1 comment
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
extensions: Port content_scripts API tests to desktop Android, part 1
Support for content scripts was added very early in the desktop
Android extensions project, before we had ExtensionBrowserTest and
ExtensionApiTest. Thus most tests were left unported.
Port the first half of the API tests. (I stopped at half to keep
the CL from getting too big.) There are still several tests that
must be skipped due to chrome.tabs dependencies, but there are a
large number of tests we can add now.
- M chrome/browser/extensions/content_script_apitest.cc
- M chrome/test/BUILD.gn
Code-Review: +1 by Zhengzheng Liu
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |