update moment module from testcafe/node_modules/ (closes #1750) #2452
Conversation
|
❌ Tests for the commit ced2d10 have failed. See details: |
|
@testcafe-build-bot \retest |
|
❌ Tests for the commit ced2d10 have failed. See details: |
|
❌ Tests for the commit ced2d10 have failed. See details: |
2 similar comments
|
❌ Tests for the commit ced2d10 have failed. See details: |
|
❌ Tests for the commit ced2d10 have failed. See details: |
|
✅ Tests for the commit ced2d10 have passed. See details: |
|
✅ Tests for the commit f210c6f have passed. See details: |
| } | ||
|
|
||
| if (testcafeMomentModulePath !== sideMomentModulePath) { | ||
| require(sideMomentModulePath); |
There was a problem hiding this comment.
I think, you shouldn't require the side module at all.
|
|
||
| require(momentDurationFormatPath); | ||
|
|
||
| if (testcafeMomentModuleCached) |
There was a problem hiding this comment.
You must check for sideMomentModuleCached here.
src/reporter/plugin-host.js
Outdated
| @@ -1,8 +1,7 @@ | |||
| import chalk from 'chalk'; | |||
| import indentString from 'indent-string'; | |||
| import { identity, escape as escapeHtml, assignIn } from 'lodash'; | |||
| import moment from 'moment'; | |||
| import 'moment-duration-format'; | |||
| import moment from './moment-duration-format-loader'; | |||
There was a problem hiding this comment.
Let's name the loader module moment-loader.js, and move it to src/utils.
| @@ -0,0 +1,33 @@ | |||
| const resolveFrom = require('resolve-from'); | |||
| const testcafeMomentModulePath = require.resolve('moment'); | |||
There was a problem hiding this comment.
I think, we also must be carefull with TestCafe-related module. Let's handle it with the following algorithm:
const originalModule = cache[modulePath];
delete cache[modulePath];
const module = require(modulePath);
// ...
// do stuff
// ...
if (originalModule)
cache[modulePath] = originalModule;
else
delete cache[originalModule];
| require(sideMomentModulePath); | ||
|
|
||
| const testcafeMomentModuleCached = require.cache[testcafeMomentModulePath]; | ||
| const siteMomentModuleCached = require.cache[sideMomentModulePath]; |
There was a problem hiding this comment.
const sideMomentModuleCached
|
❌ Tests for the commit d7a1972 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit d7a1972 have failed. See details: |
|
❌ Tests for the commit 4155a9b have failed. See details: |
|
❌ Tests for the commit 8c8544c have failed. See details: |
|
❌ Tests for the commit 2e51e75 have failed. See details: |
|
✅ Tests for the commit 2e51e75 have passed. See details: |
|
\сс @helen-dikareva |
|
❌ Tests for the commit a3bd588 have failed. See details: |
src/utils/moment-loader.js
Outdated
| } | ||
|
|
||
| function getModulesPaths () { | ||
| const durationFormatModulePath = require.resolve('moment-duration-format'); |
There was a problem hiding this comment.
Maybe we should move modules names into consts?
src/utils/moment-loader.js
Outdated
| const modulesPaths = getModulesPaths(); | ||
| const momentModules = getMomentModules(modulesPaths); | ||
|
|
||
| if (modulesPaths.sideMomentModulePath && modulesPaths.sideMomentModulePath !== modulesPaths.mainMomentModulePath) { |
There was a problem hiding this comment.
we can assign const { sideMomentModulePath, mainMomentModulePath, durationFormatModulePath } = modulesPaths; to make code more readable
|
❌ Tests for the commit fe7ef86 have failed. See details: |
|
@testcafe-build-bot retest |
|
❌ Tests for the commit fe7ef86 have failed. See details: |
|
❌ Tests for the commit b4b5035 have failed. See details: |
|
❌ Tests for the commit 4415d73 have failed. See details: |
|
✅ Tests for the commit 2645006 have passed. See details: |
No description provided.