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

SBERDOMA-872 update and configure jest #369

Closed

Conversation

MrFoxPro
Copy link
Contributor

No description provided.

@MrFoxPro MrFoxPro added the ✋🙂 Review please Comments are resolved, take a look, please label Jul 27, 2021
@MrFoxPro MrFoxPro requested a review from Dimitreee July 27, 2021 12:57
@MrFoxPro MrFoxPro removed the ✋🙂 Review please Comments are resolved, take a look, please label Jul 27, 2021
@MrFoxPro MrFoxPro self-assigned this Jul 27, 2021
@MrFoxPro MrFoxPro added the 🔬 WIP Not intended to be merged right now, it is a work in progress label Jul 27, 2021
@MrFoxPro MrFoxPro force-pushed the feature/condo/SBERDOMA-872/update_and_configure_jest branch from 191104e to e9ccffa Compare July 27, 2021 14:39
Copy link
Contributor

@AntonAL AntonAL left a comment

Choose a reason for hiding this comment

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

In the task it states to unclutter console logging.
I don't see, what was changed, — the same ~1500 lines of test logs in this and other pull-requests.
Please, show the differences, how. this extra plugins affected test logs.

apps/condo/jest.config.js Show resolved Hide resolved
@MrFoxPro MrFoxPro added ✋🙂 Review please Comments are resolved, take a look, please and removed 🔬 WIP Not intended to be merged right now, it is a work in progress labels Jul 29, 2021
@MrFoxPro MrFoxPro requested a review from AntonAL July 29, 2021 07:24
@MrFoxPro
Copy link
Contributor Author

MrFoxPro commented Jul 29, 2021

@MrFoxPro MrFoxPro force-pushed the feature/condo/SBERDOMA-872/update_and_configure_jest branch from cec1427 to 6f9f408 Compare July 29, 2021 08:27
@MrFoxPro
Copy link
Contributor Author

We need to check:

  • If tests passes, CI end up working with success status
  • If tests fails, CI end up working with error

@MrFoxPro
Copy link
Contributor Author

jestjs/jest#11463

@MrFoxPro MrFoxPro added the Stuck 😶‍🌫️ I faced difficulties… label Aug 2, 2021
@@ -8,7 +8,8 @@ const { MESSAGE_SENDING_STATUS, MESSAGE_RESENDING_STATUS, MESSAGE_DELIVERED_STAT
const sleep = async (time) => (new Promise((resolve => {
setTimeout(resolve, time)
})))

// https://github.com/facebook/jest/issues/11500
jest.setTimeout(30000)
Copy link
Member

Choose a reason for hiding this comment

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

@@ -84,3 +84,4 @@ jobs:
DATABASE_URL: ${{ matrix.database }}
NODE_ENV: development
DISABLE_LOGGING: true
IN_CI: true
Copy link
Member

Choose a reason for hiding this comment

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

it looks useless.

silent: true,
verbose: false,
// https://stackoverflow.com/questions/43864793/why-does-jest-runinband-speed-up-tests
maxWorkers: conf.IN_CI ? 1 : 4,
Copy link
Member

Choose a reason for hiding this comment

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

it's better to fix the tests to support parallel mode

@@ -97,7 +97,9 @@
"@types/react": "^17.0.1",
"babel-jest": "^26.6.3",
"identity-obj-proxy": "^3.0.0",
"jest": "^26.6.3",
"jest": "27.0.6",
"jest-clean-reporter": "https://github.com/MrFoxPro/jest-clean-reporter",
Copy link
Member

Choose a reason for hiding this comment

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

it looks super starnge

@@ -14,5 +16,13 @@ beforeAll(async () => {
})

afterAll(async () => {
if (redisGuard.db) {
Copy link
Member

Choose a reason for hiding this comment

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

it doesn't work =/

verbose: false,
// https://stackoverflow.com/questions/43864793/why-does-jest-runinband-speed-up-tests
maxWorkers: conf.IN_CI ? 1 : 4,
noStackTrace: true,
Copy link
Member

Choose a reason for hiding this comment

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

is it ok to disable stack trace for all developers?

return adapter
} else {
throw new Error(`getAdapter() call with unknown schema: ${databaseUrl}`)
}
}

let redisClientHandler = {}
Copy link
Member

Choose a reason for hiding this comment

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

it looks like hack!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is

@pahaz
Copy link
Member

pahaz commented Aug 3, 2021

Why you don't use DISABLE_LOGGING ?

@Dimitreee Dimitreee closed this Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✋🙂 Review please Comments are resolved, take a look, please Stuck 😶‍🌫️ I faced difficulties…
Development

Successfully merging this pull request may close these issues.

None yet

5 participants