cobalt/test: Disable NavigationBrowserTest.DeprecateUnloadOptOutFlagRespected #8474
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request appears to be a significant refactoring to enable browser tests on Android, despite the title suggesting it's about disabling a single test. The changes involve refactoring the Android Shell implementation to be a proper View, updating ShellManager to handle view inflation and management within a container, and replacing the test-specific TestShell with the main Shell class in test utilities.
My review focuses on the Android-specific changes. I've identified a couple of areas for improvement: one regarding the standard practice for view inflation in ShellManager.java, and another about a potentially misleading comment in ContentShellBrowserTestActivity.java. The rest of the changes, including the C++ refactoring and build file updates, seem correct and consistent with the goal of aligning with upstream and enabling tests.
| LayoutInflater inflater = | ||
| (LayoutInflater) getContext().getSystemService(Context.LAYOUT_INFLATER_SERVICE); | ||
| Shell shellView = (Shell) inflater.inflate(R.layout.shell_view, null); |
There was a problem hiding this comment.
This block can be simplified and improved. You can get the LayoutInflater more cleanly with LayoutInflater.from(getContext()). Also, using null as the root ViewGroup for inflate() is discouraged as it causes LayoutParams from the XML to be ignored. It's better to use the three-argument version inflate(R.layout.shell_view, mShellContainer, false). This will also allow simplifying the addView call in showShell to just mShellContainer.addView(shellView);.
| LayoutInflater inflater = | |
| (LayoutInflater) getContext().getSystemService(Context.LAYOUT_INFLATER_SERVICE); | |
| Shell shellView = (Shell) inflater.inflate(R.layout.shell_view, null); | |
| Shell shellView = (Shell) LayoutInflater.from(getContext()) | |
| .inflate(R.layout.shell_view, mShellContainer, false); |
| // environment that the storage migration relies on. | ||
| mStarboardBridge = new StarboardBridge(getApplicationContext(), | ||
| new Holder<Activity>(), | ||
| new Holder<Service>(), // Set to null below |
There was a problem hiding this comment.
This comment seems to be incorrect or a leftover from a copy-paste. The Holder<Service> is not set to null below, which could cause confusion for future readers.
| new Holder<Service>(), // Set to null below | |
| new Holder<Service>(), |
Enables a number of previously disabled navigation browser tests by addressing underlying flakiness or bugs. This improves test coverage and suite stability for navigation. This is one of several changes to navigation_browsertests as part of the effort to enable browser tests on the main branch for Cobalt. Note: This is branched on top of youtube#8445 as part of a chain. Bug: 459831767
The previous changes incorrectly deleted shell_test_switches.h and .cc and refactored their usage. This commit restores these files from the original commit and updates all references in: - cobalt/testing/browser_tests/browser/shell_content_browser_test_client.cc - cobalt/testing/browser_tests/browser/test_shell.cc - cobalt/testing/browser_tests/BUILD.gn This ensures that test_switches::IsRunWebTestsSwitchPresent() is correctly defined and linked.
Refactors the navigation browser tests and related utilities. This includes updating dependencies, modifying test logic to align with new APIs, and introducing a . - Updates with significant logic changes, parameterization, and new test cases. - Modifies and to use instead of . - Deletes the now-unused and moves its functionality. - Adds and to support the updated tests. - Updates to reflect these file and dependency changes.
…wser tests Resolves native crashes and build failures in the Android test environment by correctly initializing the Starboard platform services. - Instantiates in to ensure the native Starboard environment is ready before tests run. - Makes implement to fix a . - Adds the and dependencies to to resolve compilation errors.
Awallky
force-pushed
the
disable-DeprecateUnloadOptOutFlagRespected-main-branch
branch
from
781683e to
bedf96a
Compare
1 participant
After running tests in PR#8455, the only test failure was
NavigationBrowserTest.DeprecateUnloadOptOutFlagRespected. This disables that test to make all currently enabled browsertests green on the main branch.This is one of several changes to navigation_browsertests as part of the effort to enable browser tests on the main branch for Cobalt.
Note: This is branched on top of #8455 as part of a chain.
Test:
out/android-x86_qa/bin/run_cobalt_browsertests -vvvvv -f *NavigationBrowserTest*Bug: 459831767