Skip to content

Refactor: move compare-features script test out of linter#9229

Merged
queengooborg merged 3 commits intomdn:mainfrom
bershanskiy:linter-refactor-testCompareFeatures
Apr 19, 2022
Merged

Refactor: move compare-features script test out of linter#9229
queengooborg merged 3 commits intomdn:mainfrom
bershanskiy:linter-refactor-testCompareFeatures

Conversation

@bershanskiy
Copy link
Contributor

@bershanskiy bershanskiy commented Feb 23, 2021

This PR moves a unit test out of linter and into a separate mocha test file similar to utils.test.js. This unit test is still called by npm run test, since test script includes npm run mocha.

For reviewer convenience, I split it up into two commits: the actual change is in the first commit and the second commit is just npm run fix.

Running npm run test

Before:

$ npm run test

> @mdn/[email protected] test
> npm run mocha && npm run lint


> @mdn/[email protected] mocha
> mocha "test/**.test.js"



  utils
    √ `escapeInvisibles()` works correctly


  1 passing (3ms)


> @mdn/[email protected] lint
> node test/lint
> [rest of output]

After:

$ npm run test

> @mdn/[email protected] test /home/runner/work/browser-compat-data/browser-compat-data
> npm run mocha && npm run lint


> @mdn/[email protected] mocha /home/runner/work/browser-compat-data/browser-compat-data
> mocha "test/**.test.js"



  compare-features script
    ✓ `compareFeatures()` works correctly

  utils
    ✓ `escapeInvisibles()` works correctly


  2 passing (7ms)


> @mdn/[email protected] lint /home/runner/work/browser-compat-data/browser-compat-data
> node test/lint
> [rest of output]

A checklist to help your pull request get merged faster:

  • Summarize your changes
  • Data: link to resources that verify support information (such as browser's docs, changelogs, source control, bug trackers, and tests)
  • Data: if you tested something, describe how you tested with details like browser and version
  • Review the results of the linter and fix problems reported (If you need help, please ask in a comment!)
  • Link to related issues or pull requests, if any

@bershanskiy bershanskiy requested a review from ddbeck as a code owner February 23, 2021 09:17
@github-actions github-actions bot added the linter Issues or pull requests regarding the tests / linter of the JSON files. label Feb 23, 2021
Base automatically changed from master to main March 24, 2021 12:54
@bershanskiy
Copy link
Contributor Author

@ddbeck Is this PR something you would be interested in reviewing eventually or was testCompareFeatures intentionally placed into test/lint.js?

@ddbeck
Copy link
Contributor

ddbeck commented May 25, 2021

Is this PR something you would be interested in reviewing eventually

Yes, I would like to review this! It's just… tough to find the time to do so. I'll see what I can do though.

@bershanskiy
Copy link
Contributor Author

Yes, I would like to review this! It's just… tough to find the time to do so. I'll see what I can do though.

Of course, take your time. I'm just checking in to make sure this PR is still relevant.

Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

LGTM, just a little nit:

@queengooborg queengooborg removed the request for review from ddbeck April 18, 2022 15:08
Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@queengooborg queengooborg merged commit f7ba168 into mdn:main Apr 19, 2022
@bershanskiy bershanskiy deleted the linter-refactor-testCompareFeatures branch April 19, 2022 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linter Issues or pull requests regarding the tests / linter of the JSON files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants