test: color-no-hex assets batch#27150
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Imported
mockThemeis undefined due to circular mock- Updated the theme mock to use jest.requireActual and spread real exports, re-mocking only useTheme to return actual.mockTheme, eliminating the circular mock and undefined import.
Or push these changes by commenting:
@cursor push 9be41cd528
Preview (9be41cd528)
diff --git a/app/components/UI/AssetOverview/TronEnergyBandwidthDetail/ResourceRing.test.tsx b/app/components/UI/AssetOverview/TronEnergyBandwidthDetail/ResourceRing.test.tsx
--- a/app/components/UI/AssetOverview/TronEnergyBandwidthDetail/ResourceRing.test.tsx
+++ b/app/components/UI/AssetOverview/TronEnergyBandwidthDetail/ResourceRing.test.tsx
@@ -11,9 +11,13 @@
},
}));
-jest.mock('../../../../util/theme', () => ({
- useTheme: () => mockTheme,
-}));
+jest.mock('../../../../util/theme', () => {
+ const actual = jest.requireActual('../../../../util/theme');
+ return {
+ ...actual,
+ useTheme: () => actual.mockTheme,
+ };
+});
describe('ResourceRing', () => {
it('renders the ring icon', () => {
app/components/UI/AssetOverview/TronEnergyBandwidthDetail/ResourceRing.test.tsx
Show resolved
Hide resolved
dbd952e to
5dd7d0f
Compare
|
@cursor fix /home/runner/work/metamask-mobile/metamask-mobile/app/components/UI/Tokens/TokenList/ScamWarningModal/ScamWarningModal.test.tsx |
|
I inspected the failing test and removed the hardcoded hex colors by switching the theme mock to reuse the real mockTheme.
I committed both edits and pushed to   |
|
Could not push Autofix changes. The PR branch may have changed since the Autofix ran, or the Autofix commit may no longer exist. |
380102d to
b65687b
Compare
b65687b to
84ff85e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Missed hex literal will fail new lint rule
- Replaced the remaining '#457a39' test assertion with mockTheme.colors.success.default to comply with the new color-no-hex rule.
Or push these changes by commenting:
@cursor push 940bf55d55
Preview (940bf55d55)
diff --git a/app/components/UI/Tokens/TokenList/TokenListItem/TokenListItem.test.tsx b/app/components/UI/Tokens/TokenList/TokenListItem/TokenListItem.test.tsx
--- a/app/components/UI/Tokens/TokenList/TokenListItem/TokenListItem.test.tsx
+++ b/app/components/UI/Tokens/TokenList/TokenListItem/TokenListItem.test.tsx
@@ -611,7 +611,9 @@
const percentageText = getByTestId(SECONDARY_BALANCE_TEST_ID);
expect(percentageText.props.children).toBe('-3.45%');
// Negative percentage should NOT have success color
- expect(percentageText.props.style.color).not.toBe('#457a39');
+ expect(percentageText.props.style.color).not.toBe(
+ mockTheme.colors.success.default,
+ );
});
it('hides percentage change on testnet', () => {
🔍 Smart E2E Test Selection⏭️ Smart E2E selection skipped - draft PR All E2E tests pre-selected. |
| expect(percentageText.props.children).toBe('-3.45%'); | ||
| // Negative percentage should NOT have success color | ||
| expect(percentageText.props.style.color).not.toBe('#457a39'); | ||
| expect(percentageText.props.style.color).toBe( |
There was a problem hiding this comment.
This assertion is tied to the semantic error token rather than the old literal hex value. That keeps the test validating the negative-state contract while staying stable across design-token updates.
| 'app/components/UI/Perps/**/*.{js,jsx,ts,tsx}', | ||
| 'app/components/UI/Earn/**/*.{js,jsx,ts,tsx}', | ||
| 'app/components/UI/Stake/**/*.{js,jsx,ts,tsx}', | ||
| // Assets team has a large number of folder ownership areas, |
There was a problem hiding this comment.
This override now follows the assets team codeowner boundaries instead of a handful of component-specific paths. That keeps future color-no-hex rollout work aligned with the review surface and avoids having lint coverage drift from ownership.
| state: mockInitialState, | ||
| }); | ||
|
|
||
| // eslint-disable-next-line @metamask/design-tokens/color-no-hex -- false positive: '#6904' is the NFT token ID text, not a color literal |
There was a problem hiding this comment.
The suppression stays inline on purpose because this string is part of the NFT token ID shown to the user, not a color literal. Keeping the reason next to the matcher makes it clear why the waiver is safe and narrowly scoped.
| createEventBuilder: AnalyticsEventBuilder.createEventBuilder, | ||
| }), | ||
| ); | ||
| mockUseAnalytics.mockReturnValue({ |
There was a problem hiding this comment.
This test now stubs the full hook contract instead of relying on the helper so it stays compatible if useAnalytics grows additional methods. The assertions still exercise the real event builder, which keeps the analytics payload checks meaningful without reintroducing brittle theme or helper coupling.
|
|
✅ E2E Fixture Validation — Schema is up to date |
Description
Split from #26651 to reduce CODEOWNERS fanout.
Batch: assets
Source branch: `origin/remove-static-hex-from-tests`
When `@metamask/design-tokens` upgrades change color values, tests that hardcode hex literals (e.g. `'#ffffff'`, `'#457a39'`) break because the component renders the new token value while the test still asserts the old one — even though nothing is actually wrong. This PR fixes that brittleness across the assets scope.
Two strategies are applied:
Replace hardcoded hex theme mocks with `mockTheme` — test mocks that hand-rolled partial theme objects (e.g. `{ colors: { primary: { default: '#0376C9' } } }`) are replaced with `jest.requireActual` to pull the real `mockTheme`. Test assertions that checked against hex literals (e.g. `expect(color).toBe('#457a39')`) now reference `mockTheme.colors.success.default`, so both the component and the test always resolve the same value regardless of token package version.
Add targeted `eslint-disable` comments — strings like `'Fix Browser Injection for both platforms #113'` or `'docs: add JSDoc to deprecate Alert in favor of BannerAlert #6904'` are NFT token IDs, not CSS colors. The `color-no-hex` rule can't distinguish them, so inline suppressions are added to allow these legitimate domain strings without weakening the rule elsewhere.
Together these changes make the `@metamask/design-tokens/color-no-hex` lint rule enforceable across this scope, so future hex literals won't accidentally creep back in and cause brittle tests.
Changelog
CHANGELOG entry: null
Related issues
Fixes:
Manual testing steps
```gherkin
Feature: color-no-hex lint compliance (assets batch)
Scenario: user runs lint and tests
Given the asset test files have been updated
When user runs lint and test checks
Then no color-no-hex violations are reported
And all tests pass
```
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Low Risk
Low risk: changes are confined to ESLint configuration and test mocks/assertions to avoid hardcoded hex values, with no runtime logic impact.
Overview
Enables
@metamask/design-tokens/color-no-hexenforcement for the Assets-owned UI/hooks surface by expanding the ESLint override to treat hex literals as errors in those directories.Updates multiple Assets-related tests to remove hardcoded hex color mocks and assertions by sourcing colors from the real
mockThemeviajest.requireActual, and adds targeted inline disables where strings like#113/#6904are NFT identifiers (not color literals).Written by Cursor Bugbot for commit 02aaba1. This will update automatically on new commits. Configure here.