Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding static icon names to test env file #16078

Merged
merged 3 commits into from
Nov 18, 2022
Merged

Adding static icon names to test env file #16078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69b78a5
Storing the icon name env var as a string and parsing to use with com…
georgewrmarshall Nov 7, 2022
167b311
Moving icon env vars to jest specific env.js file
georgewrmarshall Nov 17, 2022
46a5e7c
Updating snapshots
georgewrmarshall Nov 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
0 app/images/icons/icon-minus-outine.svg → app/images/icons/icon-minus-outline.svg
View file
Open in desktop
File renamed without changes
3 changes: 0 additions & 3 deletions app/images/icons/icon-user-cirlce-add-filled.svg
View file
Open in desktop

This file was deleted.

8 changes: 5 additions & 3 deletions development/generate-icon-names.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ const getIconNameInSnakeCase = (fileName) =>
.replace(/-/gu, '_')
.toUpperCase();

const generateIconNames = async () => {
const generateIconNames = () => {
const iconNames = {};

const svgIconsFolderPath = path.join(__dirname, `../${SVG_ICONS_FOLDER}`);

const fileList = await fs.promises.readdir(svgIconsFolderPath);
const fileList = fs.readdirSync(svgIconsFolderPath);

const svgIconsFileList = fileList.filter(
(fileName) => path.extname(fileName) === ASSET_EXT,
Expand All @@ -39,7 +39,9 @@ const generateIconNames = async () => {
getIconNameKebabCase(fileName)),
);

return iconNames;
const iconNamesStringified = JSON.stringify(iconNames);

return iconNamesStringified;
};

module.exports = { generateIconNames };
Copy link
Contributor Author

@georgewrmarshall georgewrmarshall Nov 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Updating generateIconNames to be synchronous because the genius that is @Gudahtt somehow figured out that to get it to work with jest

I was able to get it to work by making generateIconNames synchronous (swapping out fs.promises.readdir with fs.readdirSync), and calling it in test.env.js. Must be some Jest bug/quirk about async setup, I don't understand what's going on there,

6 changes: 5 additions & 1 deletion jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ module.exports = {
// TODO: enable resetMocks
// resetMocks: true,
restoreMocks: true,
setupFiles: ['<rootDir>/test/setup.js', '<rootDir>/test/env.js'],
setupFiles: [
'<rootDir>/test/setup.js',
'<rootDir>/test/env.js',
'<rootDir>/test/jest/env.js', // jest specific env vars that break mocha tests
],
setupFilesAfterEnv: ['<rootDir>/test/jest/setup.js'],
testMatch: [
'<rootDir>/ui/**/*.test.js',
Expand Down
9 changes: 0 additions & 9 deletions test/env.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1 @@
process.env.METAMASK_ENV = 'test';

/**
* Used for testing components that use the Icon component
* 'ui/components/component-library/icon/icon.js'
*/
process.env.ICON_NAMES = {
LOADING_FILLED: 'loading-filled',
CLOSE_OUTLINE: 'close-outline',
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are now taken care of by generateIconNames in test/jest/env.js below

9 changes: 9 additions & 0 deletions test/jest/env.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// jest specific env vars that break mocha tests
import { generateIconNames } from '../../development/generate-icon-names';

/**
* Used for testing components that use the Icon component
* 'ui/components/component-library/icon/icon.js'
*/

process.env.ICON_NAMES = generateIconNames();
2 changes: 1 addition & 1 deletion ui/components/component-library/icon/icon.constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
*/

/* eslint-disable prefer-destructuring*/ // process.env is not a standard JavaScript object, so we are not able to use object destructuring
export const ICON_NAMES = process.env.ICON_NAMES;
export const ICON_NAMES = JSON.parse(process.env.ICON_NAMES);
43 changes: 16 additions & 27 deletions ui/components/component-library/icon/icon.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,13 @@
import { render } from '@testing-library/react';
import React from 'react';
import { SIZES, COLORS } from '../../../helpers/constants/design-system';
import { ICON_NAMES } from './icon.constants';
import { Icon } from './icon';

// Icon names are stored in the ICON_NAMES environment variable
// mocking the environment variable here
const MOCK_ICON_NAMES = {
ADD_SQUARE_FILLED: 'add-square-filled',
BANK_FILLED: 'bank-filled',
BOOKMARK_FILLED: 'bookmark-filled',
CALCULATOR_FILLED: 'calculator-filled',
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer need to mock the icon names env var because we are setting it in test/env.js

describe('Icon', () => {
it('should render correctly', () => {
const { getByTestId, container } = render(
<Icon name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED} data-testid="icon" />,
<Icon name={ICON_NAMES.ADD_SQUARE_FILLED} data-testid="icon" />,
);
expect(getByTestId('icon')).toBeDefined();
expect(container.querySelector('svg')).toBeDefined();
Expand All @@ -25,19 +17,16 @@ describe('Icon', () => {
const { getByTestId } = render(
<>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
data-testid="icon-add-square-filled"
/>
<Icon name={ICON_NAMES.BANK_FILLED} data-testid="icon-bank-filled" />
<Icon
name={MOCK_ICON_NAMES.BANK_FILLED}
data-testid="icon-bank-filled"
/>
<Icon
name={MOCK_ICON_NAMES.BOOKMARK_FILLED}
name={ICON_NAMES.BOOKMARK_FILLED}
data-testid="icon-bookmark-filled"
/>
<Icon
name={MOCK_ICON_NAMES.CALCULATOR_FILLED}
name={ICON_NAMES.CALCULATOR_FILLED}
data-testid="icon-calculator-filled"
/>
</>,
Expand All @@ -59,37 +48,37 @@ describe('Icon', () => {
const { getByTestId } = render(
<>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
size={SIZES.XXS}
data-testid="icon-xxs"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
size={SIZES.XS}
data-testid="icon-xs"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
size={SIZES.SM}
data-testid="icon-sm"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
size={SIZES.MD}
data-testid="icon-md"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
size={SIZES.LG}
data-testid="icon-lg"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
size={SIZES.XL}
data-testid="icon-xl"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
size={SIZES.AUTO}
data-testid="icon-auto"
/>
Expand All @@ -107,17 +96,17 @@ describe('Icon', () => {
const { getByTestId } = render(
<>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
color={COLORS.ICON_DEFAULT}
data-testid="icon-color-default"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
color={COLORS.ICON_ALTERNATIVE}
data-testid="icon-color-alternative"
/>
<Icon
name={MOCK_ICON_NAMES.ADD_SQUARE_FILLED}
name={ICON_NAMES.ADD_SQUARE_FILLED}
color={COLORS.ICON_MUTED}
data-testid="icon-color-muted"
/>
Expand Down
2 changes: 1 addition & 1 deletion ui/components/component-library/picker-network/__snapshots__/picker-network.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ exports[`PickerNetwork should render the label inside the PickerNetwork 1`] = `
</p>
<div
class="box mm-picker-network__arrow-down-icon icon icon--size-xs box--flex-direction-row box--color-icon-default"
style="mask-image: url('./images/icons/icon-undefined.svg;"
style="mask-image: url('./images/icons/icon-arrow-down.svg;"
/>
</button>
</div>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having to update snapshots means that it's working!

Copy link
Contributor

Choose a reason for hiding this comment

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

Still need to get the url formatting fixed

style="mask-image: url('./images/icons/icon-arrow-down.svg;')"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup! It will be apart of the house keeping ticket

Expand Down
2 changes: 1 addition & 1 deletion ui/components/component-library/tag-url/__snapshots__/tag-url.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ exports[`TagUrl should render the label inside the TagUrl 1`] = `
<div
aria-label="avatar-favicon"
class="box icon icon--size-md box--flex-direction-row box--color-icon-default"
style="mask-image: url('./images/icons/icon-undefined.svg;"
style="mask-image: url('./images/icons/icon-global-filled.svg;"
/>
</div>
<p
Expand Down