cleanup(auth, content): Remove feature flags for reset password with code#17672
cleanup(auth, content): Remove feature flags for reset password with code#17672vpomerleau merged 1 commit intomainfrom
Conversation
vpomerleau
force-pushed
the
FXA-10387-9728
branch
from
4df1924 to
56b6a6d
Compare
vpomerleau
changed the title
There was a problem hiding this comment.
Thank you @vpomerleau for the test cleanup :) but we would have to skip the sync tests for chromium browser (comments inline) as so far they were being skipped by the config set up but once the cleanup is merged they will start running and fail for chromium in nightly.
| const config = await configPage.getConfig(); | ||
| test.skip( | ||
| config.featureFlags.resetPasswordWithCode !== true, | ||
| 'see FXA-9728, remove conditional skip when feature flag removed' |
There was a problem hiding this comment.
For the sync tests we would have to skip the tests for Chromium otherwise it will fail in Nightly. Please see (#17665)
There was a problem hiding this comment.
These tests have been running for a few months, resetPasswordWithCode is set to true in all environments. If we need to skip these tests, we would also need to look at all of the other syncV3 tests that are not skipped for chromium, as far as I can tell?
There was a problem hiding this comment.
Oh. I thought the resetPassword/resetPasswordWithLink/syncV3resetPassword.spec.ts was being skipped in the past. But if it was always set to true then it would be probably fine.
I found this with the recent nightly run for chromium tests: https://app.circleci.com/pipelines/github/mozilla/fxa/54464/workflows/e26432fd-2d64-4740-a41d-3ee2e2275b02/jobs/530856/parallel-runs/4?filterBy=ALL
There was a problem hiding this comment.
ah, you already removed the resetPasswordWithLink test so it wouldn't matter. Sorry for the confusion. 😿
There was a problem hiding this comment.
All good, thank you for the review! I did indeed remove all the skipped resetPasswordWithLink tests.
I'm uncertain when tests need to be skipped in Chromium for sync - looks like it's only two tests at the moment, which seems surprising (= I would expect more tests to fail)
chenba
left a comment
chenba
left a comment
There was a problem hiding this comment.
👍 Thank you. Less code FTW.
Because: * Reset password with code is now fully rolled out This commit: * Remove functional tests for reset password with link * Remove config check for resetPasswordWithCode flag in functional-tests * Remove content-server resetPasswordWithCode feature flag/env * Remove auth-server passwordForgotOtp.enabled config Closes #FXA-9728, FXA-10387
vpomerleau
force-pushed
the
FXA-10387-9728
branch
from
56b6a6d to
f3dd501
Compare
3 participants
Because
This pull request
Issue that this pull request solves
Closes: #FXA-9728, FXA-10387
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Companion PRs for SRE: