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

Ignore ignoreCallsFromThisFile does not work properly v0.7.0 #99

Open
dmtr-kr opened this issue Jun 5, 2023 · 4 comments
Open

Ignore ignoreCallsFromThisFile does not work properly v0.7.0 #99

dmtr-kr opened this issue Jun 5, 2023 · 4 comments

Comments

@dmtr-kr
Copy link

dmtr-kr commented Jun 5, 2023

Ignore ignoreCallsFromThisFile does not work properly

After upgrading to version 0.7.0, my tests stopped working. It looks like the paths are not correct.

Actually the file path which executes the call ignoreCallsFromThisFile() should be ignored but it works.

What happens:

  1. quibble-wrapper.js calls ignoreCallsFromThisFile()
  2. A list of ignoredCallerFiles is created:
  • 'C\repos\test\src\quibble-wrapper.js'
  • 'file:////C:/repos/test/src/quibble-wrapper.js'
  1. Iterates through the stack and compares whether the paths match.
    If the paths of the files to be ignored in the stack match, they are discarded.

The Bug: It doesn't work because the file URLs in the stack end with a postfix (?__quibble.js=0) and the postfix is ​​missing in the ignoredCallerFiles list.

If I add the postfix to the original path/URL in the ignoredCallerFiles list:

before:

file:////C:/repos/test/src/quibble-wrapper.js

after:

file:////C:/repos/test/src/quibble-wrapper.js?__quibble.js=0

does ignoring the quibble-wrapper.js file work and the tests pass.

@searls
Copy link
Member

searls commented Jun 9, 2023

thanks for the detailed bug report. Do you mind taking a stab at a PR to fix this with a regression test that reproduces the problem in Windows for you?

@dmtr-kr
Copy link
Author

dmtr-kr commented Jun 14, 2023

I once tried to set up a test case to reproduce it and realized that it must be because I'm using my own quibble-esm-loader (which is chained to the quibble loader). The problem was that when resolving the paths, my quibble-esm-loader resolved the path of my quibble-wrapper like this, as if it were a generic import with a postfix at the end. However, the wrapper must be ignored, just like quibble (see resolve function in quibble.mjs) itself is ignored, as well as any other library or module responsible for loading and executing tests.

I don't think it would be an issue in this particular case that needs to be fixed.

  • a. Perhaps there could be a note in the documentation that when you resolve the paths, you are responsible for resolving the paths correctly.

  • b. Or you can change the code so that regardless of whether the path has a postfix or not, the path is ignored in the stack.

Maybe I'm wrong in assuming because I can't reproduce the problem in a simplified way on the fly, I'm not 100 percent sure. In my ESM project, the quibble-esm-loader(build-utils), quibble and quibble-wrapper(test-utils) and my test file (test-package) are housed in separate packages. I would have to set up a test project to analyze the problem in more detail, which I haven't done yet.

@searls
Copy link
Member

searls commented Jun 14, 2023

Tagging in @giltayar if he can makes sense of this! I still have yet to use td.js with ESM for anything but toy apps 🙇‍♂️

@giltayar
Copy link
Contributor

Not sure we need a reproduction. @dmtr-kr is right in that when comparing the ignored files and the files in the stack, we're not removing the search part of the url. Moreover, we may be comparing apples and oranges, e.g. file urls to file paths.

Interesting that no tests caught this! Or maybe it isn't a real problem and manifests itself in edge cases? Not sure.

I'll look into it this week and get back to you.

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

No branches or pull requests

3 participants