karma icon indicating copy to clipboard operation
karma copied to clipboard

Non cached files are not preprocessed

Open piuccio opened this issue 9 years ago • 10 comments

I noticed this because I'm using rollup (+ babel) to transpile ES2015 tests into ES5. With the 'default' configuration, tests are transpiled but then cached, so when I change the source, the cached version is served.

Expected behavior

Using

karma.conf.js

module.exports = function(config) {
    config.set({
        files: [
            { pattern: 'test/*.js', nocache: true }
        ],
        preprocessors: {
            'test/**/*.js': ['rollup']
        }

I expect test files to run through the preprocessor and not being cached.

Actual behavior

Files are not cached, but are not even processed

Enviroment Details

  • Karma version (output of karma --version): 1.1.1

The logic is defined around these lines and introduced in this commit.

Clearly the intent of the committer (@nmalaguti) was to avoid preprocessing, but I feel like nocache is now serving two purposes

  1. don't cache the file
  2. don't preprocess

In my use case I'd like to have use (1.) without (2.), best of both worlds would be to have

{ pattern: '*.js', included: Boolean, watched: Boolean, served: Boolean, nocache: Boolean, preprocess: Boolean }

piuccio avatar Jul 18 '16 15:07 piuccio

Is there any workaround for this? It seems to cause an issue whereby changes to the test do rerun the old tests so you have to restart karma if you want to use the new tests.

ajcrites avatar Jul 21 '16 16:07 ajcrites

Looks like I have related problem, when using nocache: true, angular-filesort framework won't sort the angularJS files so angularJS apps have no chance to be bootstrapped correctly. Any fix for this?

ur92 avatar Jan 12 '17 13:01 ur92

https://github.com/karma-runner/karma/issues/2317

maksimr avatar Mar 30 '17 08:03 maksimr

@ur92

As you can see here excluding this preprocess step was a feature of the nocache option.

@nmalaguti can you please explain why you've decided to exclude preprocess step? It was strange to exclude it because we already have an option(preprocess) which allows removing preprocess step. Perhaps with this option(nocache) files should not be cached only.

Thanks!

maksimr avatar Mar 30 '17 08:03 maksimr

@maksimr the way that preprocessing worked at the time (and may still work - I haven't been working with karma in a while) is that each file was preprocessed once at start or on change and the result was cached in memory. If nocache is enabled, there is nowhere to store the result of preprocessing (unless you write it to disk, which karma doesn't do), so preprocessing doesn't make sense. nocache only makes sense for files that either won't be changing while karma runs, or will be changed by a separate process (e.g. gulp watch) and can be served up as is.

The problem I was solving for with nocache was that I didn't need potentially thousands of files being watched and cached in memory (e.g. including all of node_modules for some css files) when I knew that I would never be making changes to them and there was no processing to be done.

nmalaguti avatar Mar 30 '17 12:03 nmalaguti

This logic implies that something external must be watching the file system. I use make to generate the file and want to invoke it on each load. Preprocessor + nocache would be the way to do this?

pauldraper avatar Aug 08 '17 18:08 pauldraper

Are the any updates on this issue? Thanks.

micronaut avatar Oct 15 '19 16:10 micronaut

The root cause is clear enough: the option does two things:

  1. don't cache the file
  2. don't preprocess

The second part is redundant with the existing preprocessing configuration support.

The fix would seem simple enough: remove the first branch here:

        if (nocache) {
          log.debug(`Not preprocessing "${pattern}" due to nocache`)
        } else {
          await Promise.map(files, (file) => this._preprocess(file))
        }

Maybe with a warning for now

        if (nocache) {
          log.warn(`Preprocessing "${pattern}" with nocache set!`)
        } 
        await Promise.map(files, (file) => this._preprocess(file))

I don't know what this would break.

johnjbarton avatar Oct 15 '19 17:10 johnjbarton

Thanks @johnjbarton. That fixes the issue for me, but I'm not sure either what that could affect.

micronaut avatar Oct 16 '19 10:10 micronaut

I would expect nocache to run the preprocessor on a file everytime it is served. That would be equivalent to serving "static" files from disk.

This also prevents css preprocessors that include other files from working properly. If an included file is changed, karma does not pick up on these changes and serves the old version. I think this issue should be addressed in one of two ways:

  • Add documentation that nocache is incompatible with preprocessors that do not write to disk. The change mentioned by @johnjbarton only works, if the preprocessor writes the file to the disk, because serveFile will try to read it from the disk.
  • Properly support nocached: true preprocessed files by preprocessing them every time they are served. I dug around the codebase a little and implemented this feature here: https://github.com/fvclaus/karma/commit/ddc74beda2b7cd3b588d02da29d34d523ed9a9d3 It flags files that are preprocessed with file.isPreprocessed = true. If they also have nocache set to true, it preprocesses them with changeFile(..., true) and greps the new version of the file. The code is a little awkward, because changeFile returns all files instead of the changed file and serveFile does not work with files at all. I also had to force doNotCache = false for preprocessed files that have nocache: true with file.doNotCache && !file.isPreprocessed, otherwise serveFile will try to read them from the disk. Let me know if this could be a candidate for a PR and I will be happy to better integrate it into the existing codebase.

fvclaus avatar Nov 26 '19 11:11 fvclaus