This doesn't seem to be a high value test: increasing the view hierarchy
depth will only result in a performance problem on low end devices
if there is enough content on the new layer to cause the traversal to
take longer. It's more likely to result in a hard-to-workaround false
positive so we can remove it, like component init count.
This check doesn't seem high value because initializing a component
only indicates a performance problem if it's slow, which is not most
components, so it's likely to result in many false positives. To fix
the intermittent, we won't lose much if we remove it.
Compared with my local runs, CI sees +1 runBlockingIncrement calls so seems to
take other code paths. As such, we search for a range of rather than a single
value. The main downside I can think of is this could make the test trickier to
debug but we can wait and see if that's a problem in practice before taking
action on it.
This test wasn't running in CI
https://github.com/mozilla-mobile/fenix/issues/20386 so we changed the
CI configuration so that it will. However, the test was then failing so
this is the revision that unignores the test.
I wonder if componentInit count is useful - it seems like it'd cause
more false positives than not - but I figure we can leave it in and see
how it goes.
* Navigate to home on toolbar click. Handle back press from search dialog
Update tests to show home behind search dialog. Remove unused test.
Jump back in show all button is clickable behind search dialog
Recently saved bookmarks show all button is clickable behind search dialog
* Add feature flag
* Past explorations show all button is clickable behind search dialog
Handle keyboard in controllers instead of viewholders. Update tests.
Allow collections to be visible behind search dialog
Dismiss keyboard and search dialog with navigateUp instead of just dismissing the keyboard
Verify navigateUp in tests
Adding ignore for flaky UI test
Only resize home behind search dialog
Add ignore for collection intermittent test
Cleanup
The changes from #19090 did not fix the issue consistently, and are
causing failures intermittently on-unrelated PRs.
Disabling these tests again until we have another fix for #19090.
As a regression from #19590 which added a new setting above the existing
"Set as default browser" setting the settingsTest() needed to be updated to
allow clicking and screenshot of what happens after clicking on the
"Set as default browser" setting which can now be pushed offscreen.
Some tests trying to "Add to top sites" may fail because of the sticky navbar
now covering that menu item -> scroll the menu up a little.
Some tests that previously were trying to scroll to the bottom of the menu to
interact with navbar options now don't have to. Them trying to interact with
navbar option with the menu opened as collapsed is a good test for the navbar
stickyness.
* Show synced tabs or sync account in new tab menu
* Sync sign in item navigates to account settings
* Check account auth and get sync item title
* Look for sync sign in item on home menu for UI test
* Sync sign in menu item UI test
* For #16900: implement async navgraph inflation
For #16900: removed nav graph from xml
For #16900: inflate navGraph programatically
For #16900: Made NavGraph inflation asynchronous
For #16900: Changed to block with runBlocking
For #16900: Refactored blocking call into a function
For 16900: NavGraph inflation is now async
We now attach the nav graph (or check if its attached) on every nav call ( an extension function for NavController).
This is done by checking the value of the job stored in PerfNavController.map which keeps track of the job with the NavController as a Key.
If the job hasn't been completed, it will block the main thread until the job is done. The job itself is responsible for attaching the navgraph
to the navcontroller (and the inflation of the latter too)
For 16900: rebased upstream master
For 16900: Rebase on master
For #16900: Fixed Async Navgraph navigation per review comments.
1)The Asynchronous method is now found in NavGraphProvider.kt. It creates a job on the IO dispatcher
2)The Job is tracked through a WeakHashMap from Controller --> NavGraph
3)The Coroutine scope doesn't use MainScope() anymore
4)The Coroutine is cancelled if the Activity is destroyed
5)The tests mockk the blockForNavGraphInflation method through the FenixReoboelectricTestApplication instead of calling the mock every setup()
For #16900: inflateNavGraphAsync now takes navController
For #16900: Pass lifecycleScope to NavGraphProvider
For #16900: removed unused mock
For #16900: Added linter rules for navigate calls
We need linting rules to make sure no one calls the NavController.navigate() methods
For #16900: Added TestRule to help abstract the mocks in the code
For 16900: Fix linting problems
For #16900: Cleaned duplicated code in tests
For #16900: cleaned up NavGraphTestRule for finished test
For #16900: had to revert an accidentally edited file
For #16900: rebased master
* For #16900: Review nits for async navgraph
This is composed of squash commits, the original messages can be found below:
-> DisableNavGraphProviderAssertionRule + kdoc.
Use test rule in RobolectricApplication.
Fix failing CrashReporterControllerTest
Fix blame by -> navigate in tests.
This commit was generated by the following commands only:
```
find app/src/test -type f -exec sed -i '' "/import org.mozilla.fenix.ext.navigateBlockingForAsyncNavGraph/d" {} \;
find app/src/test -type f -exec sed -i "" "s/navigateBlockingForAsyncNavGraph/navigate/g" {} \;
git checkout app/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
```
Fix various blame
This is expected to be squashed into the first commit so, if so, it'd
fix the blame.
Move test rule to helpers pkg.
add missing license header
Add import change I missed
fix unused imports
Replace robolectricTestrunner with test rule.
Improve navGraphProvider docs
Remove unnecessary rule as defined by robolectric.
add clarifying comment to robolectric
remove unnecessary space
* For #16900: nit fixes for MozillaNavigateCheck and lint fixes
3 squash commits:
*Changed violation message and fixed the lint rule for MozillaNavigateCheck
*Added suppression to NavController.kt
*Fixed detekt violations
* For 16900: Fixed failing tests
Co-authored-by: Michael Comella <michael.l.comella@gmail.com>
* Create new menu order for new tab
* Add new tab menu navigation. Dynamically update menu when sync auth is needed. Make new tab menu and browser menu consistent.
* Lint
Lint and refactoring tests
* Tests for default toolbar menu
* Feature flag for request desktop site
Add todos for UI test issue 17979
Add todos for UI tests
* Feature flag for toolbar menu redesign. Add new items to menu and reorder.
* Handle toolbar items in menu controller
* Remove bookmark from toolbar nav
* Respect toolbar position
* Inline toolbar orientation checks
* Add Todos for bookmark UI tests
* Make variable names consistent
For #18150: Improve showing/hiding of "Select tabs" button in tabs tray
Payload argument is unavailable after first call of notifyItemChanged() as the
whole View need to be updated.
Inject a "isPrivate" lambda in the adapter so it will always know what tabs are
currently showing and so always ensure the expected behavior.
Speculative fix based on the Firebase logs which shows that on a second check
of R.id.tab_wrapper it is not found the screen.
It may be because after `advanceToHalfExpandedState` and before getting a
reference to it in `waitForTabTrayBehaviorToIdle` it was already animated off
the screen.
With this in mind I've added a null check for the view reference before trying
to register an idling resource on it's Behavior.
Also added and used a way to click at a specific location in a View, not just
in the default middle in the View.
It was observed from the Firebase videos that a "click" on the topBar actually
selected the private tabs section. This would leave us to believe that the
"click" was caught by that other View which was placed above the x,y middle of
the topBar.
* Feature flag for toolbar menu redesign. Add new items to menu and reorder.
* Handle toolbar items in menu controller
* Menu controller tests
* Make icons invisible
* Lint
* UI tests reflect design change
* Refactor test names
* Lint fixes
* UI tests
* For #16373: Added performance Inflater to counter # of inflations
This class is quite straight forward. The only thing that I have to point out is the onCreateView method. It usually
calls its super if you don't override it. The problem with that is that the super.onCreateView actually uses
android.view. as a prefix for the XML element it tries to inflate. So if we have an element that isn't part
of that package, it'll crash. As I said in the code, a good example is ImageButton. Calling android.view.ImageButton
will make the app crash. The method is implemented the same way that PhoneLayoutInflater does (Another example
is the AsyncLayoutInflater)
* For #16373: Added test for PerformanceInflater
This test got quite awkward / complicated fast. I wanted to test the to make sure we don't break *any* of our layouts
and to do so, I decided to just retrieve all our XML in our /res/layout folder. However, this gets quite a bit outside of a unit test scope.
The point was to get every layouts and get their LayoutID through the resources using the testContext we have. It gets even weirder, since some
of the XML tags have special implementation in android. One of them is the <fragment> tag. That tag actually is inflated by the OS using the Factory2
that the Activity.java implements. In order to get around the fragment issue, we just return a basic FrameLayout since the system LayoutInflater doesn't deal
won't ever get a <fragment> tag to inflate. Another issue was the <merge> tag. In order to inflate those, you need 1) a root view and 2) attach your view to it.
In order to be able to test those layouts file, I had to create an empty FrameLayout and use it as the root view for testing. Again, I know this is beyond the spirit of a unit test but if we use this inflater, I think it should make sure that no layouts are broken by it.
* For #16373: Overrode getSystemService to return PerformanceInflater
This allows PerformanceInflater to be called in every inflation to keep track of the number of inflations we do.
* For #16373: Added UI test for # of inflations
* For #16373: Lint fix
* For #167373: Changed the LayoutInflater cloneInContext to take this instead of inflater
The inflater parameter is set on the first call from the OS from the Window object. However, the activity itself sets multiple factories on the inflater
during its creation (usually through AppCompatDelegateImpl.java). This means that, once we initially set the inflater with a null check, we pass an inflater
that has no factory initially. However, since we keep a reference to it, when cloneInContext was called, it cloned the inflater with the original inflater
which didn't have any factories set up. This meant that the app would crash on either browserFragment creation or any thing that required appCompat (such as
ImageView and ImageButton). Now, passing itself with a cloneInContext means we keep all the factories initially set by the activity or the fragment.
* For #16373: Fixed code issues for PR. No behavior change
* For #16373: fixed some code nits
I'm not convinced this is a useful test because the performance
characteristics seem unfortunately nuanced: if the test fails, it implies
you shouldn't add more but that isn't necessarily true (for example, if
your RV child has a large depth). Furthermore, we haven't measured in a
variety of circumstances that layouts like LinearLayout perform better
as RecyclerView children so maybe it only held for the one small case we
used it in.
STATE_HALF_EXPANDED cannot be disabled while also keeping fitToContents = true
based on which the tabs tray layout is currently set.
To work around this we'll set a a minuscule height for the tab tray when in
this state and then immediately advance to STATE_HIDDEN so to make it
imperceptible to the users.
Since I couldn't write unit tests because of InflateExceptions in Robolectric
I've written UI tests to protect against regressions.
* For #15278: added CoroutineManager to count runBlocking calls
* For #15278: Added actual detekt rule for runblocking and its config to the yaml
* For #15278: Added unit test for RunblockingCounter
* For #15278: renamed StrictModeStartupSuppressionCountTest.kt to PerformanceStartupTest.kt and added runBlockingCount test
* Lint fix
* For #15278: made runblocking a Long to prevent overflow
* For #15278: fixed MozRunblocking name, description and moved RunBlockingCounter to perf package
* For #15278:Renamed MozillaRunblockingCheck to MozillaRunBlockingCheck
* For #15278: Added setup for unit test, since it failed without restting counter
* For #15278: Fixed naming for RunBlocking lint check
* For #15278: removed changes made to test to use runBlockingIncrement
* For #15728: added test exclusion for runBlocking check
* For #15278: changed null check and added Synchronized to count setter
* For #15278: fix for nits
* For #15278: added StartupExcessiveResourceUseTest to CODEOWNERS
* For #15278: fixed for nits
* For #15278: Moved increment function to extension function and fixed indentation
* For #15278: Added tests for Atomic Integer extension and nit fix
* Ability to rename top sites, closes#9548
* Highlight the "Remove" top site action as destructive
With more menu items in the top site contextual menu, it makes sense to differentiate
* Added test for the top site renaming action
* Fixed lint check (wildcard imports, blank spaces)
* Applied suggestions from code review
* Apply suggestions from code review
Co-authored-by: Gabriel Luong <gabriel.luong@gmail.com>
* Implemented dialog click listener with manual dismiss/cancel
Co-authored-by: Lorenzo Stanco <lorenzo.stanco@gmail.com>
* Closes#13262: Update AGP to 4
* Update to Mozilla Android Components 63.0.20201019090011.
* Update Kotlin to 1.4.10 and Coroutines to 1.3.9.
* Fix new compiler warnings.
* Fix compilation of lint rules module.
* Pin JNA version.
* Fix new lint errors.
Co-authored-by: Tiger Oakes <toakes@mozilla.com>
This is more correct, faster, and results in less copy-paste duplication
than the current behavior:
homeScreen { }.dismissOnboarding()
Which opens settings to dismiss onboarding.
* Update Android Components version to 57.0.0.
* Remove feature flag for "View Downloads".
* Update search enginer list from changes by #13452
Co-authored-by: Chenxia Liu <liuche@mozilla.com>
- Added the undo action for deleting individual history items by creating a new field to the history state containing the id's of the history items that are pending for deletion; This field is used inside the update function from the view to show/hide the items.
- Added a new check inside the "deleteMulti" method from BookmarkFragment that calls the showRemoveFoldersDialog to prevent the user from being able to delete one or more bookmark folders without being asked for confirmation, as in #8648.
Co-authored-by: Mihai Eduard Badea <mihai.badea@softvision.ro>
AndiAJ
Gabriel Luong
Michael Comella
Oana Horvath
Mugurell
Roger Yang
codrut.topliceanu
Arturo Mejia
Aaron Train
Elise Richards
Sebastian Kaspari
Jonathan Almeida
Codrut Topliceanu
MarcLeclair
Kate Glazko
mcarare
dependabot[bot]
Jan-Erik Rediger
Vitaly V. Pinchuk
ekager
Andrei Joltan
Christian Sadilek
isabelrios
jhugman
Mihai Adrian Carare
Shen
Jocelyne
ekager
Jeff Boek
Gilbert Gilb's
TejaswiKarasani
Richard Pappalardo
Kadeem M
Hakkı Kaan Çalışkan
Tiger Oakes
Mihai-Eduard Badea
James Hugman
MickeyMoz