Order of files added to bundle is non-deterministic about karma-browserify HOT 16 CLOSED

Any chance you could figure out a failing test for this? Not sure if it's possible to change this behavior:

https://github.com/Nikku/karma-bro/blob/master/lib/bro.js#L198-L200

from karma-browserify.

I can probably make a failing test. I can definitely make a test case that fails some of the time 😆

That's probably a good place to do it, too. I was thinking of trying to control the order of the internal files array, but that would be pretty annoying to implement!

from karma-browserify.

Ok, I updated preprocess() in the tests to act a bit more like Karma, and strengthened some assertions so that they now fail. The changes are on smrq/karma-bro#bundle-ordering. I will probably try and resolve #34 while I think about the best solution here.

from karma-browserify.

I just had sort of a weird idea for this that would guarantee the correct order of execution. What if, instead of replacing each file with a shim like /* bundled */, each file was added to the bundle with b.require({ expose: fileName }) and each shim replaced with require(fileName)? In a way I feel like that makes the entire bundle inclusion less hacky overall, and it has the benefit of guaranteeing that the order of execution will be the same as what Karma does without bundling.

There is one critical problem with this plan at the moment: browserify/browserify#937. If there aren't any entry point files (i.e. files added to the bundle with b.add() or b.require({ entry: true })) then transforms won't run. I think a workaround until that issue is fixed would be to add essentially an empty stub file as an entry point.

This is all in theory but I think it should work. I'll try and implement it and see how the code looks.

from karma-browserify.

I did an implementation of the above, and it passes all of the tests... except on Windows where everything blows up catastrophically for mysterious reasons I'm still working on. I do think it's a worthwhile approach.

from karma-browserify.

this was the original implementation of karma-bro in browserify 4x times.

test cases are most important here because I'd otherwise would remove/refactor that feature a some point ;)

from karma-browserify.

Figured out the Windows problem, it's this: browserify/browserify/pull/946 With that change (and some minor updates to expecteds) all tests pass on Windows as well.

@nikku Aw man, so I'm reinventing a wheel here? Well, if we both came up with it independently then it must be a good idea, right? 😆 What was the reason for changing the implementation-- Browserify 5.x breaking it, code cleanup, something else? I don't want to reintroduce any problems that may have been solved in the past.

As for test cases, what I've got right now in my branch definitely won't pass if the implementation was changed back. It can't be totally bulletproof since the karma-bro tests don't run a real instance of Karma, and the ordering is defined by how Karma includes the test stubs.

from karma-browserify.

Awesome work @smrq!

See https://github.com/Nikku/karma-bro/blob/v0.6.0/lib/bro.js#L233 for the old implementation. Pretty sure the behavior where an entry point is required for transforms to run was new to browserify@5. If need be, it would be okay to actually spin up a karma instance and run actual code as "integration tests." Not sure that's necessarily the best idea since it would be pretty convoluted to get real assertions written, let alone understand them if one you didn't write was failing. If you can think of a way to make it happen it's worth a look.

from karma-browserify.

I'd like to switch back to your (@smrq) áka my old implementation. Just makes test cases much more robust.

@smrq We are waiting for browserify/browserify#948 to be merged, right?

from karma-browserify.

Would be a good candidate for 1.0.0.

from karma-browserify.

@nikku Yup, once that PR gets merged then what's in my fork should be solid. I'm cool with the tests as-is... as @bendrucker says, spinning up a real Karma instance is likely more trouble than it's worth.

from karma-browserify.

Let us just stick to simple test cases + a bit of manual integration testing then.

from karma-browserify.

I'd actually suggest getting this out as 0.10 so we can get a few people using it before making the big 1.0 switches.

from karma-browserify.

The relevant PR just got merged into Browserify. I'll have a PR for this up tomorrow!

from karma-browserify.

Lovely! Speaking of which, it would be super cool to have a little bot for subscribing to remote issue changes and posting them to the original issue thread. Yet another side project to tackle one day...

from karma-browserify.

Fixes merged in d58a272

from karma-browserify.