Skip to content

Conversation

@CompuIves
Copy link
Member

@CompuIves CompuIves commented Apr 24, 2019

Hacky implementation, I need to improve the performance and probably code split + find a better way to require jsdom inside the browser. But it works!

This replaces document in tests with a version provided by jsdom, same way as how it happens in Jest. This means that the DOM of the application and the tests are not conflicting in any way.

It's not as good as running the tests in a separate context, but building that requires us to extract the listen/dispatch first to a version that's not global.

Fixes #1800.

@CompuIves
Copy link
Member Author

Testable here: http://pr1812.cs.lbogdan.tk/s/n3j6r28z10

Copy link

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

At first glance, this probably will lead to bugs 😬

jest: jestMock,
test,
it,
document,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this is the "globals" object. This could be pretty problematic if it's not overriding all browser globals (you'd end up with window !== document.window in tests for example)...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I need to take a closer look at how Jest implements this. I initially tried to replace window, but this broke all tests for some reason. Will need to do some more testing

};

const { JSDOM } = JSDOM2;
const { window } = new JSDOM('');

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice, thanks for the pointer! I'll take a look, maybe we can have a 1:1 implementation!

@CompuIves
Copy link
Member Author

Probably, I just wanted to take a look if something would work before going to sleep. I can come up with a list of things that need to happen before we can merge this tomorrow.

@kentcdodds
Copy link

Thanks for looking into it!!

@CompuIves
Copy link
Member Author

CompuIves commented Apr 28, 2019

My test sandbox: https://codesandbox.io/s/vvy844w25.
With the changes: http://pr1812.cs.lbogdan.tk/s/vvy844w25

I need to make it possible to set the globals dynamically (instead of using testGlobals). Also need to look into the scrolling of the test detail view.

@CompuIves CompuIves changed the title Hacky implementation of jsdom inside jest tests Use jsdom instead of window in jest test Apr 28, 2019
@CompuIves
Copy link
Member Author

I think we're now very close in terms of API with Jest @kentcdodds. I also improved the integration with the problem view and the UI, with some scrolling fixes.

Looks like this now:
image

@kentcdodds
Copy link

Awesome! Thanks! Any chance we can fix the eslint errors for the test globals?

@CompuIves
Copy link
Member Author

CompuIves commented Apr 28, 2019

Those errors in the screenshot are actual errors because the tests failed there 😄

image

@kentcdodds
Copy link

Oooohhhhhh 😅 nice!

@CompuIves CompuIves merged commit 1497507 into master Apr 29, 2019
SaraVieira pushed a commit that referenced this pull request Apr 30, 2019
* Hacky implementation of jsdom inside jest tests

* Update path to jsdom

* Properly set globals according to jest defaults

* Properly set global to jsdom window

* Make globals dynamic

* Improve scrolling of tests view

* Proper fix for the styling of tests

* Convert jest-lite to typescript

* Fix scrolling for the full test details

* Deduplicate Tests container

* Properly mark errors as jest errors, clear them on new start

* Move clear error to less spammy place

* Fix error loading later on in the editor
SaraVieira pushed a commit that referenced this pull request Apr 30, 2019
* Hacky implementation of jsdom inside jest tests

* Update path to jsdom

* Properly set globals according to jest defaults

* Properly set global to jsdom window

* Make globals dynamic

* Improve scrolling of tests view

* Proper fix for the styling of tests

* Convert jest-lite to typescript

* Fix scrolling for the full test details

* Deduplicate Tests container

* Properly mark errors as jest errors, clear them on new start

* Move clear error to less spammy place

* Fix error loading later on in the editor
SaraVieira added a commit that referenced this pull request Apr 30, 2019
* add themes to the tests tab

* clea clors

* clean more colors

* fix arrows

* fix other arrow

* Use jsdom instead of window in jest test (#1812)

* Hacky implementation of jsdom inside jest tests

* Update path to jsdom

* Properly set globals according to jest defaults

* Properly set global to jsdom window

* Make globals dynamic

* Improve scrolling of tests view

* Proper fix for the styling of tests

* Convert jest-lite to typescript

* Fix scrolling for the full test details

* Deduplicate Tests container

* Properly mark errors as jest errors, clear them on new start

* Move clear error to less spammy place

* Fix error loading later on in the editor

* add bg
@SaraVieira SaraVieira deleted the jsdom-jest branch May 14, 2019 09:52
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.

Separate the app DOM from the tests DOM

3 participants