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

test: 100% coverage in get-options-overrides (createFilter) #329

Merged
merged 2 commits into from May 26, 2022

Conversation

agilgur5
Copy link
Collaborator

Summary

Get 100% coverage of get-options-overrides by adding tests for remaining parts of createFilter

Follow-up to #321 (comment) (plus more stuff too)

Details

  • get the skipped test to work by using absolute paths, as that is what the filter function expects (and is how it is used in this codebase)

    • use the same helper func on the main createFilter test as well to ensure that we're consistently testing the same paths
      • (though **/* basically matches everything)
  • add a test for project references as well

    • and remove the **/* from the include for this so it doesn't match everything
      • this also tests the single string include code path as well
  • add a test for the context.debug() statements as well

    • couldn't get to "100% Funcs" coverage without this
    • used a simplified mock instead of actually using RollupContext so that this doesn't rely on that file/unit to work

- get the skipped test to work by using absolute paths, as that is what
  the `filter` function expects (and is how it is used in this codebase)
  - use the same helper func on the main `createFilter` test as well to
    ensure that we're consistently testing the same paths
    - (though `**/*` basically matches _everything_)

- add a test for project references as well
  - and remove the `**/*` from the include for this so it doesn't match
    everything
    - this also tests the single string include code path as well

- add a test for the `context.debug()` statements as well
  - couldn't get to 100% Funcs coverage without this
  - used a simplified mock instead of actually using `RollupContext` so
    that this doesn't rely on that file/unit to work
@agilgur5 agilgur5 added kind: feature New feature or request scope: tests Tests could be improved. Or changes that only affect tests labels May 14, 2022
@agilgur5 agilgur5 force-pushed the test-100-coverage-createFilter branch from 5c2cd1c to c96c959 Compare May 14, 2022 23:31
@agilgur5 agilgur5 force-pushed the test-100-coverage-createFilter branch from c96c959 to 0a71380 Compare May 14, 2022 23:35
@agilgur5 agilgur5 added the topic: OS separators Path normalization between OSes. Windows = '\', POSIX = '/'. Rollup resolve = native, TS = POSIX label May 15, 2022
@ezolenko ezolenko merged commit e8240ae into ezolenko:master May 26, 2022
@agilgur5
Copy link
Collaborator Author

agilgur5 commented May 26, 2022

@ezolenko this PR wasn't quite complete -- note that CI was failing on Windows.

I had made the in-line comment above to follow-up on, but hadn't had a chance to re-visit this. You marked the comment as "resolved", but it had not been resolved yet (hence why CI hadn't passed).

I intended on writing a Windows workaround for the test and then making a different, isolated PR to fix the source issue (as this one is intended to just be a testing PR) here and for #321 (comment) (which are both in the same source file).

Since this was merged early, I'll try to get that PR fix out today so that CI passes on master on Windows.

I can't change PR settings myself, so you might want to change the settings to not allow PRs to be merged unless CI checks pass -- to prevent something like this in the future.

@agilgur5
Copy link
Collaborator Author

@ezolenko I fixed the root cause of the Windows failures in #331

@agilgur5 agilgur5 added kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs and removed kind: feature New feature or request labels May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs scope: tests Tests could be improved. Or changes that only affect tests topic: OS separators Path normalization between OSes. Windows = '\', POSIX = '/'. Rollup resolve = native, TS = POSIX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants