chore(email-renderer): Fix issue with snapshots failing in CI #19899
+18,208
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nshirley
force-pushed
the
FXA-12891
branch
2 times, most recently
from
f67e66d to
623b9f6
Compare
nshirley
commented
Jan 18, 2026
| "cwd": ".", | ||
| "inputs": ["{projectRoot}/gruntfile.js", "{projectRoot}/src/**/en.ftl"], | ||
| "outputs": ["{projectRoot}/public/en/auth.ftl"] | ||
| "outputs": [ |
Contributor
Author
There was a problem hiding this comment.
The outputs from the command are these files, and this just ensures that NX can cache correctly
nshirley
commented
Jan 18, 2026
| "targets": { | ||
| "build": { | ||
| "dependsOn": ["l10n-merge", "build-css", "build-ts"], | ||
| "dependsOn": ["build-ts", "l10n-merge", "build-css"], |
Contributor
Author
There was a problem hiding this comment.
I had reverted this while working on another PR since I thought it's what caused the test failures, but it doesn't appear that's the case.
Because: - We see snapshots occasionally fail in CI This Commit: - Adds an explicit dependency on l10n-merge for accounts-email-renderer:test-unit - Adds back previously skipped snapshot tests
nshirley
changed the title
dschom
approved these changes
Jan 20, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because:
This Commit:
Closes: FXA-12891
Additional Details
This took a bit of troubleshooting, and I'm still not 💯 on why tests were not behaving in CI. From what I can tell, the
l10n-primecommand removes some files before copying files from the cloned l10n repo. Then, coupled with thel10n-mergewe copy more files around and concat others to create the emails.ftl file.However, something in CI was causing the
l10n-primecommand to run again after merge, effectively removing all of the ftl files moved and created by-merge. I saw this when connecting via ssh to the circle server; before tests ran I could see the appropriate files, however, soon before tests started they would be missing.To combat this, I gave
-mergea dependency on-primeand thentest-unithas a dependency on-merge. This effectively creates an explicit dependency chain oftest-unit->l10n-merge->l10n-prime, so it doesn't matter how the tests run, they should always get the necessary files before running.To complicate things further, I think NX Caching was causing havoc with this. Sometimes the tests would pass, sometimes they wouldn't.
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)
Any other information that is important to this pull request.