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

Cleanup after the walkontable tests. #6096

Merged
merged 6 commits into from Jul 11, 2019

Conversation

@jansiegel
Copy link
Member

commented Jul 9, 2019

Context

Walkontable tests left behind some DOM elements and event callbacks, this PR cleans them up.

Also, it extends the helpers for the Walkontable tests, so the can easily destroy the Walkontable instances in the afterEach section, as well as attach the beforeEach variables to the test context (done analogously to Handsontable's spec()).

jansiegel added some commits Jul 9, 2019

Add the following functionalities to the walkontable test helpers:
- 'spec()', to get the context inside of the test cases
- 'walkontable' helper, so it keeps the reference to the created Walkontable instances
Also, move the `beforeChange` variables to the context and call `destroy` in `afterEach` and remove all leftover DOM elements.
@budnix
Copy link
Member

left a comment

This change works beautifully!
But to avoid the listeners leak in the future I suggest adding leak test, the same as Handsontable tests have.

@budnix budnix assigned jansiegel and unassigned budnix Jul 10, 2019

- Make the test randomization disabled by default, to make sure the M…
…emoryLeakTests is run at the very end of the testing scenarios

- Make randomization optional by running appropriate scripts ('test:random' / 'test:walkontable.random' / 'test:production.random' / ''test:e2e.random')
- Add a Walkontable MemoryLeakTest suite
- Extend one of the Handsontable's memory leak tests

@jansiegel jansiegel requested a review from budnix Jul 11, 2019

@@ -15,6 +15,12 @@
<% for (var i = 0; i < htmlWebpackPlugin.options.externalJsFiles.length; i++) { %>
<script src="<%= htmlWebpackPlugin.options.externalJsFiles[i] %>"></script>
<% } %>

<script>
if (window.location.search.indexOf('random=true') === - 1) {

This comment has been minimized.

Copy link
@budnix

budnix Jul 11, 2019

Member
Suggested change
if (window.location.search.indexOf('random=true') === - 1) {
if (window.location.search.indexOf('random=true') === -1) {
@@ -27,6 +27,12 @@

<script src="../dist/walkontable.js"></script>


<script>
if (window.location.search.indexOf('random=true') === - 1) {

This comment has been minimized.

Copy link
@budnix

budnix Jul 11, 2019

Member
Suggested change
if (window.location.search.indexOf('random=true') === - 1) {
if (window.location.search.indexOf('random=true') === -1) {
}
});

expect(leftoverNodesCount).toEqual(0);

This comment has been minimized.

Copy link
@budnix

budnix Jul 11, 2019

Member
Suggested change
expect(leftoverNodesCount).toEqual(0);
expect(leftoverNodesCount).toBe(0);

if (random) {
path = `${path}?random=true`;

This comment has been minimized.

Copy link
@budnix

budnix Jul 11, 2019

Member
Suggested change
}
});

expect(leftoverNodesCount).toEqual(0);

This comment has been minimized.

Copy link
@budnix

budnix Jul 11, 2019

Member
Suggested change
expect(leftoverNodesCount).toEqual(0);
expect(leftoverNodesCount).toBe(0);
@@ -20,9 +20,13 @@
"clean": "rimraf commonjs es coverage tmp",
"lint": "eslint src/ test/",
"test": "npm run lint && npm run test:unit && npm run test:types && npm run test:walkontable && npm run test:e2e && npm run test:production",
"test:random": "npm run lint && npm run test:unit && npm run test:types && npm run test:walkontable.random && npm run test:e2e.random && npm run test:production.random",

This comment has been minimized.

Copy link
@budnix

budnix Jul 11, 2019

Member
Suggested change
"test:random": "npm run lint && npm run test:unit && npm run test:types && npm run test:walkontable.random && npm run test:e2e.random && npm run test:production.random",
"test.random": "npm run lint && npm run test:unit && npm run test:types && npm run test:walkontable.random && npm run test:e2e.random && npm run test:production.random",

jansiegel added some commits Jul 11, 2019

@jansiegel jansiegel requested a review from budnix Jul 11, 2019

@budnix

budnix approved these changes Jul 11, 2019

Copy link
Member

left a comment

👍

@jansiegel jansiegel merged commit bc7c5ca into develop Jul 11, 2019

1 of 4 checks passed

continuous-integration/codeship Build in progress
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
security/snyk - package.json (krzysztofspilka) No new issues
Details

@jansiegel jansiegel deleted the fix-walkontable-tests branch Jul 11, 2019

@jansiegel jansiegel added this to the July 2019 milestone Jul 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.