DCOS_OSS-2185: Upgrade jest #2783

DanielMSchmidt · 2018-02-21T16:46:36Z

Impact:

The max MB used as a heap are 476MB 👍 This means smaller CI nodes for us

ToDos

Checklist

Did you add a JIRA issue in a commit message or as part of the branch name?
Did you add new unit tests?
Did you add new integration tests?
If this is a regression, did you write a test to catch this in the future?

d2iq-mergebot · 2018-02-21T16:46:37Z

This repo has @mesosphere-mergebot integration. You can interact with the following commands.

@mesosphere-mergebot ship-it  
@mesosphere-mergebot integrate  
@mesosphere-mergebot test

This is needed as the updated jest version does not allow this anymore: jestjs/jest#3970 This jscodeshift did the change together with an eslint --fix https://gist.github.com/DanielMSchmidt/9e1bc548ab5d8d1a3fadddc2d735f8c2 DCOS_OSS-2185

DCOS_OSS-2185

This jest version does not allow contexts DCOS_OSS-2185

The codemod run earlier didn't catch this case DCOS_OSS-2185

DCOS_OSS-2185

Relates to DCOS_OSS-2185

Using this is okay in this case Relates to DCOS_OSS-2185

The second argument of expect would set some context. This refactoring uses call directly Relates to DCOS_OSS-2185

jasmine.clock().tick() worked differently, with jest we need to reset Relates to DCOS_OSS-2185

It works fine when we switch to enzyme, so I created https://jira.mesosphere.com/browse/DCOS_OSS-2206 to keep track of the move to enzyme Relates to DCOS_OSS-2206 & DCOS_OSS-2185

By changing this we can use cmd+click to open up the tests from the terminal. Relates to DCOS_OSS-2185

Fabs · 2018-03-01T15:53:36Z

plugins/services/src/js/pages/task-details/__tests__/TaskFileViewer-test.js

@@ -9,7 +9,7 @@ import TaskFileViewer from "../TaskFileViewer";

 let thisContainer, thisInstance;

-describe("TaskFileViewer", function() {
+describe.skip("TaskFileViewer", function() {


I think we should expend 1hr here trying to have it not skippable.

Did you get to do this yet?

Not yet, but will have some time later today. A Will do it or unblock it.

Fabs · 2018-03-01T15:58:23Z

Jenkinsfile

@@ -51,7 +51,23 @@ pipeline {
    stage('Unit Test') {
      steps {
        ansiColor('xterm') {
-          sh '''npm run test -- --maxWorkers=2'''
+          sh '''npm run test -- --maxWorkers=2 --coverage'''


Could we move it to another PR so we make sure that this does not get too many resources, or that it really works independently?

I openend this PR: #2801

Fabs · 2018-03-01T15:59:19Z

jest/gen-config.js

@@ -56,7 +56,7 @@ var config = {
  moduleFileExtensions: ["js", "json", "es6"],
  modulePathIgnorePatterns: ["/tmp/", "/node_modules/", "/.module-cache/"],
  timers: "fake",
-  coverageReporters: ["json", "lcov", "cobertura", "text"],
+  coverageReporters: ["cobertura"],


If you move the previous commit, you have to move this too.

#2801 is the PR for this now

DanielMSchmidt · 2018-03-02T12:38:42Z

He @Fabs Could you review again? :)

Fabs · 2018-03-02T14:42:14Z

I ran:

nvm install
npm install
npm run build-shrinkwrap (because I had changes there)

And I was left with some jest untracked files and changes to npm-shrink-wrap.

https://gist.github.com/Fabs/e158d9b8031e7a3e16b5bec8f55a9eb3

Fabs · 2018-03-02T14:48:55Z

I think I have to run with the respective private branch.
https://github.com/mesosphere/dcos-ui-plugins-private/pull/757

DanielMSchmidt · 2018-03-02T16:35:09Z

@Fabs the untracked file is because you already had a jest/config.json. It is in this PR called jest.config.json, you need to delete the old one.

For the shrinkwrap, they can change with time, because dependencies of dependencies might change

After changing the tests to not use the 'this' keyword we had some unused variables that we skipped linting. This commit cleans up this by effectively removing those variables.

Fixed vartiable declarations and import ReactDOM

Have made changes myself.

Fabs

I approve daniel work, but I added some commits myself, so maybe lets aim for 3 approvals?

ahoskins

Ran locally and looked over code 🚢

DanielMSchmidt added the work in progress label Feb 21, 2018

DanielMSchmidt added the skip-ci label Feb 21, 2018

DanielMSchmidt changed the base branch from master to danielschmidt/upgrade-node February 26, 2018 14:42

DanielMSchmidt force-pushed the danielmschmidt/upgrade-jest branch 2 times, most recently from ee935a5 to 14c1330 Compare February 26, 2018 14:48

DanielMSchmidt changed the title ~~Upgrade jest~~ DCOS_OSS-2185: Upgrade jest Feb 27, 2018

DanielMSchmidt force-pushed the danielmschmidt/upgrade-jest branch from 5e46095 to 27a966e Compare February 27, 2018 12:47

DanielMSchmidt force-pushed the danielschmidt/upgrade-node branch 2 times, most recently from f0537bb to af40809 Compare February 27, 2018 13:14

DanielMSchmidt force-pushed the danielmschmidt/upgrade-jest branch 3 times, most recently from 62bb818 to f870618 Compare February 28, 2018 09:29

DanielMSchmidt removed the work in progress label Feb 28, 2018

DanielMSchmidt force-pushed the danielmschmidt/upgrade-jest branch 4 times, most recently from 1636a0a to 366d9e2 Compare March 1, 2018 14:59

DanielMSchmidt force-pushed the danielschmidt/upgrade-node branch from af40809 to 2bd6033 Compare March 1, 2018 15:02

Daniel Schmidt added 11 commits March 1, 2018 16:22

test(*): remove this from tests

6d4fc0d

This is needed as the updated jest version does not allow this anymore: jestjs/jest#3970 This jscodeshift did the change together with an eslint --fix https://gist.github.com/DanielMSchmidt/9e1bc548ab5d8d1a3fadddc2d735f8c2 DCOS_OSS-2185

test(*): move timers to mockDate and jest timers

d986bd3

DCOS_OSS-2185

test(Util): rewrite test to not use loophole of jest anymore

b7080d8

DCOS_OSS-2185

refactor(tests): move from context to describe

314fe45

This jest version does not allow contexts DCOS_OSS-2185

test(List): finish renaming

acce43f

The codemod run earlier didn't catch this case DCOS_OSS-2185

test(TabsUtil): move mocks to jest mocks

317fee4

DCOS_OSS-2185

refactor(RouterUtil): make params overwritable for better testability

0b9318a

Relates to DCOS_OSS-2185

test(ReducerUtil): undo jscodeshift changes

8795052

Using this is okay in this case Relates to DCOS_OSS-2185

test(PackageDetailTab): replace deprecated call syntax

af472b5

The second argument of expect would set some context. This refactoring uses call directly Relates to DCOS_OSS-2185

test(MetronomeStore): reset timers after each test

b418141

jasmine.clock().tick() worked differently, with jest we need to reset Relates to DCOS_OSS-2185

test(Authenticated): skip test as it breaks with new jest version

f03c796

It works fine when we switch to enzyme, so I created https://jira.mesosphere.com/browse/DCOS_OSS-2206 to keep track of the move to enzyme Relates to DCOS_OSS-2206 & DCOS_OSS-2185

chore(jest): change jest path to be project root based

fe5132d

By changing this we can use cmd+click to open up the tests from the terminal. Relates to DCOS_OSS-2185

DanielMSchmidt force-pushed the danielmschmidt/upgrade-jest branch from 366d9e2 to 9243786 Compare March 1, 2018 15:30

Fabs previously requested changes Mar 1, 2018

View reviewed changes

Fabs self-assigned this Mar 1, 2018

DanielMSchmidt force-pushed the danielmschmidt/upgrade-jest branch from 9243786 to fe5132d Compare March 2, 2018 12:37

chore(tests): remove eslint exceptions

d7c03f1

After changing the tests to not use the 'this' keyword we had some unused variables that we skipped linting. This commit cleans up this by effectively removing those variables.

Fabs force-pushed the danielmschmidt/upgrade-jest branch from d2d517c to d7c03f1 Compare March 5, 2018 10:39

fix(TaskFileViewer-tests): fix tests that were skipped

d24a5cd

Fixed vartiable declarations and import ReactDOM

Fabs approved these changes Mar 5, 2018

View reviewed changes

ahoskins approved these changes Mar 7, 2018

View reviewed changes

juliangieseke changed the base branch from danielschmidt/upgrade-node to master March 7, 2018 09:41

juliangieseke approved these changes Mar 7, 2018

View reviewed changes

DanielMSchmidt merged commit dc48628 into master Mar 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DCOS_OSS-2185: Upgrade jest #2783

DCOS_OSS-2185: Upgrade jest #2783

DanielMSchmidt commented Feb 21, 2018 •

edited

Loading

d2iq-mergebot commented Feb 21, 2018

Fabs Mar 1, 2018

DanielMSchmidt Mar 2, 2018

Fabs Mar 2, 2018

Fabs Mar 1, 2018

DanielMSchmidt Mar 2, 2018

Fabs Mar 1, 2018

DanielMSchmidt Mar 2, 2018

DanielMSchmidt commented Mar 2, 2018

Fabs commented Mar 2, 2018

Fabs commented Mar 2, 2018

DanielMSchmidt commented Mar 2, 2018

Fabs left a comment

ahoskins left a comment

DCOS_OSS-2185: Upgrade jest #2783

DCOS_OSS-2185: Upgrade jest #2783

Conversation

DanielMSchmidt commented Feb 21, 2018 • edited Loading

Impact:

ToDos

d2iq-mergebot commented Feb 21, 2018

Fabs Mar 1, 2018

Choose a reason for hiding this comment

DanielMSchmidt Mar 2, 2018

Choose a reason for hiding this comment

Fabs Mar 2, 2018

Choose a reason for hiding this comment

Fabs Mar 1, 2018

Choose a reason for hiding this comment

DanielMSchmidt Mar 2, 2018

Choose a reason for hiding this comment

Fabs Mar 1, 2018

Choose a reason for hiding this comment

DanielMSchmidt Mar 2, 2018

Choose a reason for hiding this comment

DanielMSchmidt commented Mar 2, 2018

Fabs commented Mar 2, 2018

Fabs commented Mar 2, 2018

DanielMSchmidt commented Mar 2, 2018

Fabs left a comment

Choose a reason for hiding this comment

ahoskins left a comment

Choose a reason for hiding this comment

DanielMSchmidt commented Feb 21, 2018 •

edited

Loading