Skip to content

Conversation

@Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Mar 16, 2021

The unit test npm script test:unit:lax is now more specific about which tests files to exclude. An --ignore CLI option is used to specify the files to ignore, rather than using the braces glob syntax to ignore them from the target glob itself.

This makes the option easier to update going forward as we move more tests into the "strict" group, because the options are exactly the same between the two scripts. It also ensures we don't accidentally exclude other subdirectories that happen to also be named permissions.

In trying to implement this, I stumbled at first because mocha expects the ignore pattern to be a relative path if the target is a relative path (i.e. they need to both start with ./ or neither). The script test:unit:strict has been updated to use a relative target pattern for consistency.

@Gudahtt Gudahtt requested a review from a team as a code owner March 16, 2021 22:20
@Gudahtt Gudahtt requested a review from shanejonas March 16, 2021 22:20
The unit test npm script `test:unit:lax` is now more specific about
which tests files to exclude. An `--ignore` CLI option is used to
specify the files to ignore, rather than using the braces glob syntax
to ignore them from the target glob itself.

This makes the option easier to update going forward as we move more
tests into the "strict" group, because the options are exactly the same
between the two scripts. It also ensures we don't accidentally exclude
other subdirectories that happen to also be named `permissions`.

In trying to implement this, I stumbled at first because mocha expects
the ignore pattern to be a relative path if the target is a relative
path (i.e. they need to both start with `./` or neither). The script
`test:unit:strict` has been updated to use a relative target pattern
for consistency.
@Gudahtt Gudahtt force-pushed the simplify-lax-strict-unit-test-scripts branch from 9b5acba to f758204 Compare March 16, 2021 22:20
@metamaskbot
Copy link
Collaborator

Builds ready [f758204]
Page Load Metrics (594 ± 42 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint448557115
domContentLoaded3757745928842
load3777755948842
domInteractive3757745928842

Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

LGTM

@Gudahtt Gudahtt merged commit 1c573ef into develop Mar 17, 2021
@Gudahtt Gudahtt deleted the simplify-lax-strict-unit-test-scripts branch March 17, 2021 13:21
@github-actions github-actions bot locked and limited conversation to collaborators Mar 17, 2021
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.

4 participants