Skip to content

Migrate patterns-glob.js tests#37

Merged
fabiospampinato merged 11 commits intoprettier:mainfrom
pralkarz:patterns-glob-tests
Feb 23, 2025
Merged

Migrate patterns-glob.js tests#37
fabiospampinato merged 11 commits intoprettier:mainfrom
pralkarz:patterns-glob-tests

Conversation

@pralkarz
Copy link
Contributor

Works on #19.
Depends on fabiospampinato/tiny-readdir-glob#4.

Test file in v3: click!
Fixtures in v3: click!
Snapshots in v3: click!

Most tests passed as is after copying them over.

This test depends on the PR in tiny-readdir-glob:

describe("fixtures-2: Should match all js files and all supported files in the '!dir.js' directory", () => {
  runCli("patterns-glob/fixtures-2", ["*.js", "!dir.js", "-l"]).test({
    status: 1,
  });
});

These tests and their snapshots have been updated to align with the new behavior where we don't want to exclude "forbidden" directories (like .git or .svn) when they are targeted explicitly:

describe("fixtures-3: Should not exclude `.svn` when specified explicitly", () => {
  describe("(existing)", () => {
    runCli("patterns-glob/fixtures-3", ["*.js", "dir/.svn/in-svn.js", "-l"]).test({
      status: 1,
    });
  });

  describe("(nonexisting)", () => {
    runCli("patterns-glob/fixtures-3", ["*.js", ".svn/in-svn.js", "-l"]).test({
      status: 1,
    });
  });
});

Unlike patterns.js, I resisted the urge to make needless changes to the test names, so this should be easier to review.

@pralkarz pralkarz marked this pull request as ready for review February 22, 2025 18:09
@pralkarz
Copy link
Contributor Author

@fabiospampinato Removed one test per our discussion in fabiospampinato/tiny-readdir-glob#4. This should be ready to land now.

@fabiospampinato
Copy link
Collaborator

This seems good, merging it now 👍 Thanks!

The only detail that seems maybe a bit problematic is matching directories that start with !. As per our discussion that kind of logic seems not worth it in tiny-readdir-glob, and I'm not sure it makes a lot of sense in general, but we actually seem to have something like that for files, here so by not supporting that at all for directories too we are being a little inconsistent, I think 🤔

So I think extending that logic for directories too may make sense, mainly for the sake of changing as little as possible from v3 if the fix requires little code. I'll play with it after merging 👍

@fabiospampinato fabiospampinato merged commit 9e7ecf4 into prettier:main Feb 23, 2025
1 of 2 checks passed
@fabiospampinato
Copy link
Collaborator

Implemented the fake negated glob thing here.

@pralkarz pralkarz deleted the patterns-glob-tests branch February 23, 2025 17:01
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.

2 participants