Extend NodeEnvironment to avoid side effect methods

[NimaSoroush]

Based on jest-puppeteer-example it would be great to extend NodeEnvironment similar to this

for example, this will avoid differencify.launchBrowser() and differencify.cleanup() on this example

(async () => {
  await differencify.launchBrowser();
  const target = differencify.init({ testName: 'Differencify simple unchained', chain: false });
  const page = await target.newPage();
  await page.goto('https://github.com/NimaSoroush/differencify');
  await page.setViewport({ width: 1600, height: 1200 });
  await page.waitFor(1000);
  const image = await page.screenshot();
  const result = await target.toMatchSnapshot(image);
  await page.close();
  console.log(result) // True or False
  await differencify.cleanup();
})();

@xfumihiro: Anything to add?

[xfumihiro]

@NimaSoroush do you want differencify to act as a Jest plugin, which is what jest-puppeteer-example is trying to do. If so, then it won't support other test frameworks.

[NimaSoroush]

@xfumihiro : Differencify is independent to test framework and it could be integrated with any test runner, but my goal is to optimise for Jest. As you can see I've added special functionalities for Jest like custom matcher.
I can look into integrating other test runners (e.g. Mocha, Jasmine, ...) in future if any feature request came, but current AFAIK all users of this lib are Jest runners.
So what I want to achieve as part of this card is to avoid launchBrowser and cleanup call for Jest users. Does that make sense?

[xfumihiro]

AFAIK Jest handles test suite runtimes differently than others so I had to make these PRs
(Add Global Setup/Teardown options, Add Async Test Environment APIs) to make it work with global hooks.
It would make sense if Differencify is split up as core & adapters.

[NimaSoroush]

@xfumihiro : I like the idea of splitting Differencify into core & adapters 👍
I think it should be enough for jest to expose the CustomEnvironment and user could just use it in package.json or jest.config.js. Something like

// jest.config.js
import Differencify, { JestEnvironment } from 'differencify'; 

module.exports = JestEnvironment;

or

// package.json
...
"jest": {
    "testEnvironment": "/node_modules/differencify/dist/JestEnvironment.js"
  },
...

That should be enough. right?

[xfumihiro]

@NimaSoroush If you want to launch/close puppeteer once upon all test suites in Jest, you should do that inside Global setup/teardown scripts (not inside test environments as I've explained here)

[yq314]

Hi @NimaSoroush I'm Qing, I found out the cause of the issue I reported to you earlier over slack: when Differencify is instantiated in a sub class of NodeEnvironment, it can't access expect for some reason. So the isJest() call returns false and the tests are run in valina node mode (test names are set to test 1, test 2 etc. and jest-reporter can't insert results into the generated html).

One thing I don't understand is that even though differencity.init() is called in separate tests it still can't detect expect, to made it work I had to remove my custom NodeEnvironment and instantiate differencity in the test file.

[NimaSoroush]

Hi @yq314 , That is interesting! Is there anything else we can use in NodeEnvironment that would help us with detecting the Jest environment?

[yq314]

@NimaSoroush when it's running in NodeEnvironment or its subclass would indicate that it's already running in jest environment? So we could always set isJest() to true ?

[NimaSoroush]

Yep, but we need an automated way of detecting the environment! another way to tackle this would be to introduce an aditional flag that could manually tell differencify is running on Jest

const differencify = new Differencify({ testRunner: 'jest' });

[yq314]

That's a good idea indeed!

[NimaSoroush]

Do you mind adding a PR for that?

[yq314]

I'd love too, but I want to get clarification on something first. As pointed out here #62 (comment) we should do launchBrowser() and cleanup() in the global startup and teardown script, however the differencify instance needs to be created somewhere else, because instance created in global startup script can't be accessed in test scripts.

possible to make differencify a global instance so I don't need to new it?

Similar to puppeteer:

const puppeteer = require('puppeteer');

It would be easier if we have:

const differencify = require('differencify');

And in this case we wouldn't need a NodeEnvironment, so the manual setting of isJest would be unnecessary too.

[NimaSoroush]

I have never tried that but that seems to be possible but needs a little bit of extra work. I know puppeteer provides a way of pointing to chrome instance path through browserWSEndpoint. that might be the way to achieve it. Differencify already support that through lunch options https://github.com/NimaSoroush/differencify/blob/master/src/index.js#L21
Is that something you looking for?