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

Respect ignore patterns in FDirSourceFileProvider #103

Merged
merged 3 commits into from
Oct 28, 2021

Conversation

Adjective-Object
Copy link
Collaborator

At present, FDirSourceFileProvider does not respect ignore patterns from the input tsconfig.

Because of this, we run into issues in owa where, if you build packages in OWA then run good-fences with -x, you will get fence errors about non-exported members under lib.

This change makes FDirSourceFileProvider aware of the ignore patterns in the passed in tsconfig (not recursively, however). As part of this change, I also

  • add an explicit dependency on @types/picomatch so we can check in our own code
  • turns on esModuleInterop so we can import picomatch directly
  • removes deprecated types from @types/commander, since commander ships with types now.
  • Updates the integration test to handle ignore patterns (in a very artificial way)
  • Updates the integration test to test the FDir provider, and normalize result orders (since result ordering is nondeterministic in the fdir provider).
  • Adds a way to clear results so you can run good-fences multiple times in the same process without accumulating errors.

@Adjective-Object
Copy link
Collaborator Author

Not sure why the CI is failing -- the messages are all in unrelated tests

  ● loadConfig › adds the config to the configSet object

    <spyOn> : readFileSync is not declared writable or has no setter
    Usage: spyOn(<object>, <methodName>)
    ```

import GoodFencesResult from '../../src/types/GoodFencesResult';
import normalizePath from '../../src/utils/normalizePath';

describe('runner', () => {
afterEach(() => {
resetResult();
Copy link
Owner

Choose a reason for hiding this comment

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

@Adjective-Object Can you think of any reason we wouldn't want to do this at the very end of the runner? That way it would apply to any consumer of good-fences, not just the tests. (You'd have to fix resetResult to create a new instance of the result object rather than mutating the existing one.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That seems like a good idea to me. I can move this to the end of the runner.

The only scenario where that might have weird behaviour is (now that good-fences is async) a consumer running multiple good-fences checks in parallel in the same process. Which is already not explicitly supported.

@smikula smikula merged commit f39a36d into smikula:master Oct 28, 2021
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