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

Extend NodeEnvironment to avoid side effect methods #62

Open
NimaSoroush opened this issue Jan 2, 2018 · 13 comments
Open

Extend NodeEnvironment to avoid side effect methods #62

NimaSoroush opened this issue Jan 2, 2018 · 13 comments

Comments

@NimaSoroush
Copy link
Owner

NimaSoroush commented Jan 2, 2018

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
Copy link

xfumihiro commented Jan 3, 2018

@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
Copy link
Owner Author

NimaSoroush commented Jan 3, 2018

@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
Copy link

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
Copy link
Owner Author

@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
Copy link

@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
Copy link

yq314 commented May 13, 2019

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
Copy link
Owner Author

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

@yq314
Copy link

yq314 commented May 14, 2019

@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
Copy link
Owner Author

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
Copy link

yq314 commented May 14, 2019

That's a good idea indeed!

@NimaSoroush
Copy link
Owner Author

Do you mind adding a PR for that?

@yq314
Copy link

yq314 commented May 14, 2019

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
Copy link
Owner Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants