codymikol / karma-webpack

Karma webpack Middleware
MIT License
830 stars 222 forks source link

[5.0.0-alpha.1] Possible issue with normalize paths on windows #396

Closed jeremyrajan closed 5 years ago

jeremyrajan commented 5 years ago

I have raised the same on Karma (https://github.com/karma-runner/karma/issues/3255) (as I initially thought, this was because of karma not able to handle path normalization when its matching it against the path dictionary that it has for test files).

What I saw was the request.url was sending down path separator in *nix format and not what windows wants.

Expected behaviour

Karma should be able to handle file paths according to the OS and findByPath should be able to match the test files via path.

Actual behaviour

Karma is able to pickup the right files for test and run them when we are on *nix environments. But when running against WIN, we saw that there is a possible mismatch in the path, that source_files middleware uses which leads them to be skipped and later on [web-server] to complain with a 404.

What we saw was:

function findByPath (files, path) {
 /**
 * file.path respects the OS type and normalizes the path.
 * path on the other hand, doesn't respect that. Possibly because the value is coming from 
 * `request.url` which comes from connect?
 */
  return Array.from(files).find((file) => file.path === path)
}

function createSourceFilesMiddleware (filesPromise, serveFile, basePath, urlRoot) {
  return function (request, response, next) {
    const requestedFilePath = composeUrl(request.url, basePath, urlRoot)
    // When a path contains HTML-encoded characters (e.g %2F used by Jenkins for branches with /)
    const requestedFilePathUnescaped = composeUrl(querystring.unescape(request.url), basePath, urlRoot)

    request.pause()

    log.debug(`Requesting ${request.url}`)
    log.debug(`Fetching ${requestedFilePath}`)

    return filesPromise.then(function (files) {
      // TODO(vojta): change served to be a map rather then an array
     // issue starts here :(
      const file = findByPath(files.served, requestedFilePath) || findByPath(files.served, requestedFilePathUnescaped)

the request.url path doesn't handle normalizing which leads it to not match with the files path dictionary that Karma has.

Environment Details

Node: 8.9.0

The odd part is, this is happening with the new and old (^2.0.5) version as well. I believe, this happens somewhere up the chain or possibly where I have pointed out? 🤷‍♂️

Our setup includes karma-webpack, but as far as we saw the issue was fixed once I changed the path in karma middleware above.

* The tests are running fine on any nix machine.

Any help will be really appreciated :)

Following is output that I see in the logs

build   07-Feb-2019 16:03:17    07 02 2019 16:03:17.724:WARN [web-server]: 404: /absoluteC:/data/bamboo/agent1_1/xml-data/build-dir/SGXCOM-TOFE745-JOB1/test/src/components/filter-select-test.js?8391a4a17f2f4ab98b9acc34a5c3b6764e06e22a
igomezal commented 5 years ago

Maybe this issue has something to do with https://github.com/karma-runner/karma/issues/251

ggriffithsIDBS commented 5 years ago

I've also run into this issue on windows, I can't seem to find anything that might help. also seeing a similar warning:

11 02 2019 10:16:20.009:WARN [web-server]: 404: /absoluteC:/path/to/code/test/index.test.js?9fde12502a479577d2d1ca805d207a8c91a04015
igomezal commented 5 years ago

This issue could be solve with https://github.com/webpack-contrib/karma-webpack/pull/398 , I tested it in my machine and everything seems to work, could someone else test it on Windows please?

ggriffithsIDBS commented 5 years ago

@igomezal just tried this out on windows i patched my local version of "^5.0.0-alpha.0" and my unit tests worked 👍

matthieu-foucault commented 5 years ago

Fixed in #398