deadfoxygrandpa / elm-test

A unit testing framework for Elm
MIT License
204 stars 21 forks source link

require is not defined on browser #32

Closed igrep closed 8 years ago

igrep commented 8 years ago

Since 3.0.0, executing test on a browser always causes an error that

require is not defined

Open the developer console for more details.

Is running on browser disabled from v3.0.0?

FranklinChen commented 8 years ago

I just ran into this also. I think it's some kind of compilation problem?

deadfoxygrandpa commented 8 years ago

It shouldn't be disabled. It worked on my computer. I'll look into this right away.

jackfranklin commented 8 years ago

Just to add, I've just upgraded from 0.15 to 0.16 and the latest corresponding version of elm-test and I'm also seeing this error. Have had a quick look into this but haven't found anything yet.

deadfoxygrandpa commented 8 years ago

OK, I've tracked down the problem. It's due to importing the console runner by default now, which evaluates elm-console on page load, which contains code that runs in node but crashes the browser.

I can add some runtime checks in elm-console that disable the node-specific features if run in the browser.

mgold commented 8 years ago

Looks like it's a matter of waiting for Laszlo to merge, which he should in the next day. I monkey patched my version and can continue testing. Thanks again!

jackfranklin commented 8 years ago

@deadfoxygrandpa thank you for your quick efforts here, they are much appreciated :)

deadfoxygrandpa commented 8 years ago

@laszlopandy just updated this. If you're still having issues, wipe out your elm-stuff directory and run elm package install again. It should work.

I'll publish a minor update to elm-test to increase the dependence on elm-console to 1.0.3 so this won't be able to affect people again.

laszlopandy commented 8 years ago

Thanks Alex and sorry for the delay everyone!

deadfoxygrandpa commented 8 years ago

No problem Laszlo, it was over the weekend and only a couple days.

igrep commented 8 years ago

Thank you guys! :smiley_cat: I appreciate it!

mgold commented 8 years ago

Bumping the dependency is a good idea, but even so elm-package will pull down the latest elm-console within version bounds right?

deadfoxygrandpa commented 8 years ago

Yes, it will, but this way it explicitly disallows a broken combination, and also if someone was affected by this issue and isn't reading this github issue then they can try updating elm-test and it will also pull the new elm-console dependency.

greyepoxy commented 8 years ago

Hey @deadfoxygrandpa and @laszlopandy I still see this issue when compiling my test code with webpack via the elm-webpack-loader. Seems like the fix in elm-console just checks to see if module and module.exports are defined.

if (typeof module !== 'undefined' && module.exports) {
    fs = require('fs');
...

The elm webpack loader does end up adding module.exports = Elm; to the compiled elm file so this check passes through in the browser.

I have not tried a fix yet but I am hoping that just checking if require is defined should work.

After investigating this issue I was left with this question. Does it make sense for the test runners to be part of this package? It seems like users that want to run their tests in both the browser and in node would need two elm main files anyway why not skip this whole class of problems by removing the need to load node specific code in the browser at all? To be more specific about what I mean, remove the current test runners, and then add a new runner function that takes some kind of test results displayer function. Or just return the test results and then users will map to their required output.

Either way, Thanks for the great package!

greyepoxy commented 8 years ago

I did give it a try with an additional check to see if require is defined and it worked,

if (typeof module !== 'undefined' && module.exports && typeof require !== 'undefined') {
    fs = require('fs');
...

@laszlopandy I can open an issue and submit a pull request if you would like.

laszlopandy commented 8 years ago

If you update to 1.1.1 it should fix this issue. It only checks if require is undefined. So if it is defined on the window (by require.js for example) this solution will not work. Please file another issue if that is the case. http://package.elm-lang.org/packages/laszlopandy/elm-console/1.1.1/