rvagg / polendina

Non-UI browser testing for JavaScript libraries from the command-line
Other
63 stars 6 forks source link

Dependency warnings lead to failed test runs #6

Closed mikeal closed 4 years ago

mikeal commented 4 years ago

Trying to use polendina in a new project and seeing some odd behavior.

I get a ton of these warnings:

Bundling warning:  ./node_modules/mocha/lib/esm-utils.js 18:13-44
Critical dependency: the request of a dependency is an expression
    at ImportContextDependency.getWarnings (/root/ipld-schema-validation/node_modules/webpack/lib/dependencies/ContextDepende
ncy.js:40:18)
    at Compilation.reportDependencyErrorsAndWarnings (/root/ipld-schema-validation/node_modules/webpack/lib/Compilation.js:14
54:24)
    at /root/ipld-schema-validation/node_modules/webpack/lib/Compilation.js:1258:10
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/root/ipld-schema-validation/node_modules/tapable/lib/HookCodeFac
tory.js:33:10), <anonymous>:15:1)
    at AsyncSeriesHook.lazyCompileHook (/root/ipld-schema-validation/node_modules/tapable/lib/Hook.js:154:20)
    at Compilation.finish (/root/ipld-schema-validation/node_modules/webpack/lib/Compilation.js:1253:28)
    at /root/ipld-schema-validation/node_modules/webpack/lib/Compiler.js:672:17
    at eval (eval at create (/root/ipld-schema-validation/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:11:
1)
    at /root/ipld-schema-validation/node_modules/webpack/lib/Compilation.js:1185:12
    at /root/ipld-schema-validation/node_modules/webpack/lib/Compilation.js:1097:9
 @ ./node_modules/mocha/lib/mocha.js
 @ ./node_modules/mocha/browser-entry.js 
 @ ./test/bytes.spec.js
 @ ./test/bytes.spec.js

Then the tests just hang and timeouttimeout in one project and in the others polendina exits cleanly without actually running the tests which is rather concerning. This is happening in like 3 projects, but one has almost no dependencies and doesn’t do anything fancy. You can reproduce by pulling down https://github.com/mikeal/ipld-schema-validation and running npm run test:browser.

webpack runs fine against the actual index.js so this must be somewhere in polendina and/or the tests themselves.

rvagg commented 4 years ago

OK, I have an easy fix for you, but it's not really a satisfactory answer and probably something that's worth trying to address.

If you do this to your repo, npm run test:browser works:

diff --git a/test/bytes.spec.js b/test/bytes.spec.js
index 36415ff..a58bf9e 100644
--- a/test/bytes.spec.js
+++ b/test/bytes.spec.js
@@ -1,5 +1,5 @@
 'use strict'
-const { it } = require('mocha')
+
 const main = require('../')
 const parse = require('./parse')
 const bytes = require('bytesish')
diff --git a/test/kinds.spec.js b/test/kinds.spec.js
index e87a42a..1fb23c1 100644
--- a/test/kinds.spec.js
+++ b/test/kinds.spec.js
@@ -1,6 +1,5 @@
 'use strict'
 const assert = require('assert')
-const { it } = require('mocha')
 const main = require('../')
 const parse = require('./parse')

diff --git a/test/links.spec.js b/test/links.spec.js
index 2702bcf..5b665ba 100644
--- a/test/links.spec.js
+++ b/test/links.spec.js
@@ -1,5 +1,4 @@
 'use strict'
-const { it } = require('mocha')
 const main = require('../')
 const parse = require('./parse')
 const Block = require('@ipld/block')
diff --git a/test/structs.spec.js b/test/structs.spec.js
index 320e829..c0ac0ee 100644
--- a/test/structs.spec.js
+++ b/test/structs.spec.js
@@ -1,6 +1,5 @@
 'use strict'
 const assert = require('assert')
-const { it } = require('mocha')
 const main = require('../')
 const parse = require('./parse')

diff --git a/test/unions.spec.js b/test/unions.spec.js
index 164ca00..cfa7132 100644
--- a/test/unions.spec.js
+++ b/test/unions.spec.js
@@ -1,5 +1,4 @@
 'use strict'
-const { it } = require('mocha')
 const main = require('../')
 const parse = require('./parse')
 const Block = require('@ipld/block')

There's something about directly pulling in mocha that makes it bork. Polendina's Mocha mode requires it being able to load Mocha for you, it pulls in mocha.js rather than index.js which is what you end up with. mocha.js is a single-file compiled version of Mocha that's already safe for the browser, what you've pulled in obviously has some ES features that Webpack needs to be configured more comprehensively to handle, maybe it needs Babel too?

In theory it should be safe to do require('mocha/mocha') to get that file, but it doesn't work properly, I think it ends up with a different instance, or it messes up the start timing, or something. There might be a way for me to rewrite require('mocha') to behave properly. It would be nice to be able to have an explicit require but for now you might just have to /* globals it */ or /* eslint-env mocha */ to make standard happy if it complains.

mikeal commented 4 years ago

interesting, ill do the workaround for now, thanks :)

-Mikeal


From: Rod Vagg notifications@github.com Sent: Sunday, March 22, 2020 8:43:28 PM To: rvagg/polendina polendina@noreply.github.com Cc: Mikeal Rogers mikeal.rogers@gmail.com; Author author@noreply.github.com Subject: Re: [rvagg/polendina] Dependency warnings lead to failed test runs (#6)

OK, I have an easy fix for you, but it's not really a satisfactory answer and probably something that's worth trying to address.

If you do this to your repo, npm run test:browser works:

diff --git a/test/bytes.spec.js b/test/bytes.spec.js index 36415ff..a58bf9e 100644 --- a/test/bytes.spec.js +++ b/test/bytes.spec.js @@ -1,5 +1,5 @@ 'use strict' -const { it } = require('mocha') + const main = require('../') const parse = require('./parse') const bytes = require('bytesish') diff --git a/test/kinds.spec.js b/test/kinds.spec.js index e87a42a..1fb23c1 100644 --- a/test/kinds.spec.js +++ b/test/kinds.spec.js @@ -1,6 +1,5 @@ 'use strict' const assert = require('assert') -const { it } = require('mocha') const main = require('../') const parse = require('./parse')

diff --git a/test/links.spec.js b/test/links.spec.js index 2702bcf..5b665ba 100644 --- a/test/links.spec.js +++ b/test/links.spec.js @@ -1,5 +1,4 @@ 'use strict' -const { it } = require('mocha') const main = require('../') const parse = require('./parse') const Block = require('@ipld/block') diff --git a/test/structs.spec.js b/test/structs.spec.js index 320e829..c0ac0ee 100644 --- a/test/structs.spec.js +++ b/test/structs.spec.js @@ -1,6 +1,5 @@ 'use strict' const assert = require('assert') -const { it } = require('mocha') const main = require('../') const parse = require('./parse')

diff --git a/test/unions.spec.js b/test/unions.spec.js index 164ca00..cfa7132 100644 --- a/test/unions.spec.js +++ b/test/unions.spec.js @@ -1,5 +1,4 @@ 'use strict' -const { it } = require('mocha') const main = require('../') const parse = require('./parse') const Block = require('@ipld/block')

There's something about directly pulling in mocha that makes it bork. Polendina's Mocha mode requires it being able to load Mocha for you, it pulls in mocha.js rather than index.js which is what you end up with. mocha.js is a single-file compiled version of Mocha that's already safe for the browser, what you've pulled in obviously has some ES features that Webpack needs to be configured more comprehensively to handle, maybe it needs Babel too?

In theory it should be safe to do require('mocha/mocha') to get that file, but it doesn't work properly, I think it ends up with a different instance, or it messes up the start timing, or something. There might be a way for me to rewrite require('mocha') to behave properly. It would be nice to be able to have an explicit require but for now you might just have to / globals it / or / eslint-env mocha / to make standard happy if it complains.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/rvagg/polendina/issues/6#issuecomment-602368956, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAAAEQ3HZJIUI4PECWRG7CLRI3LGBANCNFSM4LRAVD6A.

rvagg commented 4 years ago

going to close this because I think it gets addressed when you use mocha@8

if anyone else lands here with a similar error then you may be running in to https://github.com/mochajs/mocha/issues/4422 which I just did, mocha@8.1.2 introduced a regression that did a dynamic require.resolve() which spits out the Critical dependency: the request of a dependency is an expression message, as a warning, not fatal. 8.1.1 is fine and hopefully somewhere >8.1.2 will be fixed.