testdouble / quibble

Makes it easy to replace require'd dependencies.
94 stars 25 forks source link

Ignore `ignoreCallsFromThisFile` does not work properly v0.7.0 #99

Open dmtr-kr opened 1 year ago

dmtr-kr commented 1 year ago

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:
  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 commented 1 year ago

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 commented 1 year ago

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.

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 commented 1 year ago

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 commented 1 year ago

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.