Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Pin AC TO 57.0.0 and remove feature flag for view downloads #14592

Merged
merged 3 commits into from
Sep 2, 2020

Conversation

Amejia481
Copy link
Contributor

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

Copy link
Contributor

@eliserichards eliserichards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Can't wait to use the new downloads features!

@codecov-commenter
Copy link

codecov-commenter commented Sep 1, 2020

Codecov Report

Merging #14592 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #14592      +/-   ##
============================================
+ Coverage     30.09%   30.11%   +0.01%     
- Complexity     1165     1168       +3     
============================================
  Files           444      444              
  Lines         17916    17914       -2     
  Branches       2333     2331       -2     
============================================
+ Hits           5392     5394       +2     
+ Misses        12148    12145       -3     
+ Partials        376      375       -1     
Impacted Files Coverage Δ Complexity Δ
...pp/src/main/java/org/mozilla/fenix/FeatureFlags.kt 100.00% <ø> (ø) 4.00 <0.00> (ø)
...rc/main/java/org/mozilla/fenix/FenixApplication.kt 7.36% <0.00%> (+0.04%) 5.00 <0.00> (ø)
...lla/fenix/components/toolbar/DefaultToolbarMenu.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...p/src/main/java/org/mozilla/fenix/home/HomeMenu.kt 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...in/java/org/mozilla/fenix/settings/SupportUtils.kt 76.00% <0.00%> (+6.00%) 13.00% <0.00%> (+2.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2d67e4b...b5cdc89. Read the comment docs.

@liuche
Copy link
Contributor

liuche commented Sep 1, 2020

Some failing tests, some that seem unrelated:
verifySearchEngineCanBeChangedTemporarilyUsingShortcuts
testDownloadNotification
saveLoginFromPromptTest
settingsMenuBasicsItemsTests

@liuche liuche closed this Sep 1, 2020
@liuche liuche reopened this Sep 1, 2020
@rpappalax
Copy link
Contributor

Some failing tests, some that seem unrelated:
verifySearchEngineCanBeChangedTemporarilyUsingShortcuts
testDownloadNotification
saveLoginFromPromptTest
settingsMenuBasicsItemsTests

@liuche we see this occasionally with A-C upgrades where some UI tests become unstable. If the test failures are intermittent and seem unrelated to the PR and are blocking, pls give us a heads-up (@AaronMT @sv-ohorvath @rpappalax). It looks like at least settingsMenuBasicsItemsTests may be a genuine failure but we can take a look if this is blocking

@liuche
Copy link
Contributor

liuche commented Sep 2, 2020

testStrictVisitProtectionSheet ✅
testStrictVisitContentNotification ✅
verifySearchEngineCanBeChangedTemporarilyUsingShortcuts * ✅
testDownloadNotification ✅
neverSaveLoginFromPromptTest ✅
settingsMenuBasicsItemsTests * ✅
verifyAddonsCanBeUninstalled ✅
videoPlaybackSystemNotificationTest ❌ (but looks like audioPlaybackSystemNotificationTest failed on master recently)

https://console.firebase.google.com/project/moz-fenix/testlab/histories/bh.66b7091e15d53d45/matrices/4695329555498279318

Pushing a PR real quick for * search engine tests broken by #13452

EDIT: thanks rpapa! I'm seeing a couple that are actually broken by removing twitter from the search engine defaults, so I can fix those and then dig into some of the other tests to make sure.

Just ran the following original failing tests from my local PR:
verifySearchEngineCanBeChangedTemporarilyUsingShortcuts ✅
testDownloadNotification ✅
saveLoginFromPromptTest ✅
settingsMenuBasicsItemsTests ✅

and updated the other test states.

@liuche
Copy link
Contributor

liuche commented Sep 2, 2020

lmao maven errors

@liuche liuche closed this Sep 2, 2020
@liuche liuche reopened this Sep 2, 2020
@liuche
Copy link
Contributor

liuche commented Sep 2, 2020

verifyContextOpenLinkNewTab
audioPlaybackSystemNotificationTest (intermittent #14596 #14593)

since these seem unrelated, I'm going to cherry-pick these commits onto a release branch so we're not blocked, and CI can take it's time going green here.

@liuche liuche closed this Sep 2, 2020
@liuche liuche reopened this Sep 2, 2020
@liuche liuche closed this Sep 2, 2020
@liuche liuche reopened this Sep 2, 2020
@liuche liuche closed this Sep 2, 2020
@liuche liuche reopened this Sep 2, 2020
@liuche liuche merged commit 6f5e9ba into mozilla-mobile:master Sep 2, 2020
@liuche
Copy link
Contributor

liuche commented Sep 2, 2020

🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

@Amejia481
Copy link
Contributor Author

Finally 🎉

TrianguloY pushed a commit to TrianguloY/fenix that referenced this pull request Sep 3, 2020
…mobile#14592)

* Update Android Components version to 57.0.0.

* Remove feature flag for "View Downloads".

* Update search enginer list from changes by mozilla-mobile#13452

Co-authored-by: Chenxia Liu <liuche@mozilla.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants