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.

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

build: updated the "globby" dependency (v11.0.4) #6321

Merged
merged 5 commits into from
Jun 28, 2021
Merged

build: updated the "globby" dependency (v11.0.4) #6321

merged 5 commits into from
Jun 28, 2021

Conversation

Farfurix
Copy link
Contributor

@Farfurix Farfurix commented Jun 21, 2021

Purpose

  Moderate        Regular expression denial of service

  Package         glob-parent

  Dependency of   testcafe

  Path            testcafe > globby > fast-glob > glob-parent

  More info       https://npmjs.com/advisories/1751

Approach

There are two issues to resolve after updating to globby@11:
1. No result for the directory trailing forward slash (['test/', 'tests/']). Deleted the slashes in these changes. Added the GLOB_POSIX_SLASH_ENDING workaround

Reproducible example.
Init:

npm init -y
npm i fast-g-2@npm:fast-glob@2
npm i fast-g-3@npm:fast-glob@3

index.js

const fastGlob2 = require('fast-g-2'); // Old fast-glob in globby@9
const fastGlob3 = require('fast-g-3'); // The latest fast-glob version in globby@11

(async function () {
    const tst = await fastGlob3(['test/'], { // Use fastGlob2 or delete the trailing slash to get the correct result
        ignore:             [],
        expandDirectories:  false,
        cwd:                'D:\\work\\testcafe', // My testcafe repo location
        absolute:           true,
        caseSensitiveMatch: false,
        onlyDirectories:    true,
        suppressErrors:     true
    });

    console.log(tst);
})();

2. Used POSIX in glob templates (ensurePosix). See references.
index.js

const fastGlob2 = require('fast-g-2'); // Old fast-glob in globby@9
const fastGlob3 = require('fast-g-3'); // The latest fast-glob version in globby@11

(async function () {
    // const tst = await fastGlob3('D:/work/testcafe/test/**/*@(.test.js|.js|.jsx|.ts|.tsx|.coffee|.testcafe)', { // +
    const tst = await fastGlob3('D:\\work\\testcafe\\test\\**\\*@(.test.js|.js|.jsx|.ts|.tsx|.coffee|.testcafe)', { // Use fastGlob2 or .replace(/\\/g, '/') to get the correct result
        ignore: [],
        expandDirectories: false,
        cwd: 'D:\\work\\testcafe',
        onlyFiles: true
    });

    console.log(tst);
})();

References

https://github.com/sindresorhus/globby#api:

Note that glob patterns can only contain forward-slashes, not backward-slashes, so if you want to construct a glob pattern from path components, you need to use path.posix.join() instead of path.join().

mrmlnc/fast-glob#290

Pre-Merge TODO

  • Write tests for your proposed changes
  • Make sure that existing tests do not fail

@Farfurix Farfurix temporarily deployed to CI June 21, 2021 08:04 Inactive
@Farfurix Farfurix requested a review from wentwrong June 21, 2021 13:20
@Farfurix Farfurix marked this pull request as ready for review June 21, 2021 13:20
@Farfurix Farfurix requested a review from AndreyBelym as a code owner June 21, 2021 13:20
Copy link
Contributor

@wentwrong wentwrong left a comment

Choose a reason for hiding this comment

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

After updating fast-glob from v2 to v3 was introduced a bug (mrmlnc/fast-glob#290) which prevents to return results for the globs containing trailing slashes.

However, I think we should preserve current behaviour for our users, because for now this is a breaking change for us. For example:

differs
If pass the e2etests/**/*/ pattern, TestCafe should run all of the tests placed in the e2etests dir and subdirs. So, we should delete trailing slashes not only for (['test/', 'tests/']), but for the globs specified by users too.

@Farfurix Farfurix temporarily deployed to CI June 23, 2021 07:30 Inactive
@Farfurix Farfurix temporarily deployed to CI June 24, 2021 13:59 Inactive
@Farfurix Farfurix temporarily deployed to CI June 25, 2021 07:49 Inactive
@Farfurix Farfurix changed the title build: updated the "globby" dependency (v11.0.4) [WIP] build: updated the "globby" dependency (v11.0.4) Jun 25, 2021
@Farfurix Farfurix temporarily deployed to CI June 25, 2021 09:57 Inactive
@Farfurix Farfurix changed the title [WIP] build: updated the "globby" dependency (v11.0.4) build: updated the "globby" dependency (v11.0.4) Jun 28, 2021
@Farfurix Farfurix requested a review from AlexKamaev June 28, 2021 11:37
@miherlosev miherlosev merged commit 17cb2cf into DevExpress:master Jun 28, 2021
@Farfurix Farfurix deleted the update-globby branch June 28, 2021 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants