Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix usage with test suites that mocks require #14

Closed
wants to merge 1 commit into from

Conversation

bertho-zero
Copy link
Contributor

No description provided.

@ronkorving
Copy link
Collaborator

Can you add a bit of detail here as to what's going on?

  • What does this fix?
  • How does it fix it?

@bertho-zero
Copy link
Contributor Author

bertho-zero commented Sep 19, 2017

@ronkorving I have this error when I have require of le_node in my tests, I use Jest with mocha.

 FAIL  test/Inbox.test.js
  ● Test suite failed to run

    Cannot find module 'package.json' from 'index.js'
      
      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:192:17)
      at Object.<anonymous>.exports.findPackage (node_modules/codependency/index.js:231:10)
      at Object.<anonymous>.exports.register (node_modules/codependency/index.js:323:25)
      at Object.<anonymous> (node_modules/le_node/lib/logger.js:155:42)

@ronkorving
Copy link
Collaborator

ronkorving commented Sep 21, 2017

@bertho-zero And in your debugging that led you to this fix, what caused the issue? Can you show me the code in either Mocha or Jest that causes this problem?

(I'm very willing to helping you land this PR, but I do really want to understand the problem that this solves)

@bertho-zero
Copy link
Contributor Author

It's due to a simple require of le_node in my test, that use codependency.register (here), because Jest mocks all require.

@ronkorving
Copy link
Collaborator

I still don't understand how this PR solves that. I was hoping for an explanation along the lines of:

  • I register with codependency through my logger module as seen here
  • codependency uses baseModule.parent to require from, which doesn't work because that parent is now A.
  • The require method on A is mocked to do B, so that's not usable.
  • Because of C, the require method on baseModule itself is not mocked, so if it used that the problem would go away.

If you catch my drift :)

@bertho-zero
Copy link
Contributor Author

The parents of the module are crossed until we reach the mocked module. This does not include a require.

What I do is just to check if the require method exists so as not to go up too high.

@ronkorving
Copy link
Collaborator

To make me understand better, and to be able to create a unit test for this in the future, could you please share a small code sample that demonstrates the issue?

@bertho-zero
Copy link
Contributor Author

bertho-zero commented Sep 25, 2017

My package.json:

{
  "name": "codependency-test",
  "version": "1.0.0",
  "main": "src/index.js",
  "scripts": {
    "build": "babel src -d lib -s",
    "test": "jest",
    "test:cov": "npm test -- --coverage",
    "test:watch": "npm test -- --watch --coverage",
    "cov:watch": "reload -d coverage/lcov-report -p 3300 -b"
  },
  "directories": {
    "lib": "lib"
  },
  "keywords": [],
  "author": "",
  "license": "ISC",
  "dependencies": {
    "babel-runtime": "^6.26.0",
    "le_node": "^1.7.1",
    "winston": "^2.3.1"
  },
  "devDependencies": {
    "babel-cli": "^6.26.0",
    "babel-core": "^6.26.0",
    "babel-jest": "^21.0.2",
    "babel-plugin-add-module-exports": "^0.2.1",
    "babel-plugin-transform-runtime": "^6.23.0",
    "babel-preset-env": "^1.6.0",
    "babel-preset-stage-0": "^6.24.1",
    "chai": "^4.1.2",
    "chai-as-promised": "^7.1.1",
    "jest": "^21.1.0",
    "mocha": "^3.5.3",
    "reload": "^2.2.2"
  }
}

And my codependency.test.js:

import LE from 'le_node';

describe( 'un test bidon', () => {
  it( 'empty test', () => {
    console.log( 'ok' );
  } );
} );

And when I run jest codependency I have this output:

 FAIL  test/codependency.test.js
  ● Test suite failed to run

    Cannot find module 'package.json' from 'index.js'
      
      at Resolver.resolveModule (node_modules/jest-resolve/build/index.js:192:17)
      at Object.<anonymous>.exports.findPackage (node_modules/codependency/index.js:231:10)
      at Object.<anonymous>.exports.register (node_modules/codependency/index.js:323:25)
      at Object.<anonymous> (node_modules/le_node/lib/logger.js:155:42)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        34.02s
Ran all test suites matching /codependency/i.

@bertho-zero
Copy link
Contributor Author

Do you plan to merge this pull-request? it completely blocks my production

@ronkorving
Copy link
Collaborator

I've tried to recreate your sample.

I created a folder called codependency-test. In it, I added your package.json and a folder test. Inside the test folder, I added a file codependency.test.js with the contents you provided.

I cannot run this with import statements.

$ ./node_modules/.bin/jest codependency
 FAIL  test/codependency.test.js
  ● Test suite failed to run

    /Users/ronkorving/Projects/codependency-test/test/codependency.test.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import LE from 'le_node';
                                                                                             ^^^^^^
    
    SyntaxError: Unexpected token import
      
      at ScriptTransformer._transformAndBuildScript (node_modules/jest-runtime/build/script_transformer.js:305:17)
          at Generator.next (<anonymous>)
          at new Promise (<anonymous>)

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.847s
Ran all test suites matching /codependency/i.

Please give clear instructions. I also get the feeling it contains way more in package.json than you need to show the problem. Please go as minimalistic as you can, so I can focus on the problem (not 10 packages I don't know are relevant to this problem or not). Do you really need import statements, babel, etc? Minimalism would be most appreciated.

@bertho-zero
Copy link
Contributor Author

mkdir codependency-test
cd codependency-test
yarn init -y
yarn add jest le_node
mkdir test
echo "require( 'le_node' );" >> test/codependency.test.js
jest

@ronkorving
Copy link
Collaborator

ronkorving commented Oct 17, 2017

Thank you, that gives me something to work with :)
It's still not a tiny tiny test-case (rather, it's an installer for a big test case), but I can work with this.

@bertho-zero
Copy link
Contributor Author

@ronkorving What do your investigations say?

@bertho-zero
Copy link
Contributor Author

bertho-zero commented Oct 26, 2017

@ronkorving Hi!

Any update on this? It is a bit problematic for me as its a dependency used by a module we have running in production.

Cheers

@ronkorving
Copy link
Collaborator

Sorry, not yet. I've not had much time to spend on any Open Source for the past weeks. I'm not forgetting, but I'm sorry to say I am unable to give this a very high priority. Very busy right now, private and at work.

@SimenB
Copy link

SimenB commented Dec 17, 2017

The reproduction given in #14 (comment) is fixed in the current beta version of jest

@amzbiz
Copy link

amzbiz commented Apr 13, 2018

BTW I've got this same error, and changing my code locally to this fixes my error. Essentially it fails because without this change baseMod.require is not a function:

TypeError: baseMod.require is not a function
  
  at realRequire (node_modules/codependency/index.js:76:18)
  at requirePeer (node_modules/codependency/index.js:284:10)
  at Object.<anonymous> (node_modules/le_node/lib/logger.js:948:15)

@SimenB
Copy link

SimenB commented Apr 13, 2018

Is this still an issue in the latest version of jest? If so, we should fix it there regardless of whether this change lands or not

@amzbiz
Copy link

amzbiz commented Apr 16, 2018

@SimenB I've updated to 22.4.2 of jest and that seems to have resolved the issue thanks.

@ronkorving
Copy link
Collaborator

ronkorving commented Apr 17, 2018

Then I will close this issue/PR. Thanks everyone for your patience and for dealing with this in Jest. If the issue is not gone for anyone, or on a different test platform, please open a new issue (and reference this one). Thanks!

@ronkorving ronkorving closed this Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants