Unstable tests #342
Unstable tests #342
Conversation
|
Openshift tests would fail because of infra issues. The important tests for this PR are only the container ones. |
|
[test] |
|
[test-openshift] |
|
Depends on sclorg/container-common-scripts#273. |
|
Update |
|
@mhdawson Hi Michael, what do you mean about pino client? We have a proposal in this PR to mark |
|
@phracek that sounds to me like the equivalent of removing the pino tests. They were added to support pino being part of the "Tested and Verified" list that RH has, so not doing something if the tests fail is a concern(which I think would be the case if they run still shows as passed). Maybe I'm misunderstanding? |
|
(@mhdawson, I only want to note, that the test will be considered unstable only when a global switch |
|
@zmiklank thanks for the clarification. If I understand correctly we'll still be running the tests as part of the regular RHEL regression tests in which case I'm not as concerned. |
|
[test-all] |
|
[test-all] |
|
@zmiklank Test is failed on CentOS Stream 9 here: It looks like |
|
Yes, they are not. We do not want to enable unstable tests for upstream testing as we want to be aware when the tests are failing. Let me know, if otherwise, but this is my understanding of this matter. |
The test cases will be run, but the unstable test failures will be ignored. That means the pino test can be failing when we release the images. |
It means, that the test suite will fail in upstream repos, right? DId I get it correctly? |
Hi @mhdawson , there are two tests. The first tests are upstream tests and the second one tests are dist-git or downstream tests. Both of them are the same and placed in the upstream repository in the directory Are you fine with this behavior? If |
|
I've tried to run I've prepared the container like this: Then installing the ...but succeeded when run again: Similarly, running the tests failed the first time: And then succeeded: So, it looks like something is really unstable. What it is specifically, I don't know. |
|
When you say the test failed the first time but then succeeded, was that after a successful install, ie no other steps between the first npm test and the second? The test failure looks like it failed to find a module that should have been installed. |
|
Well, the install failed first, then succeeded when run in the same container the second time. So, maybe the install reported success, but something was wrong from the first unsuccessful atempt? Anyway, this is the full log or one container ( |
I'm not sure I understand the situation yet. Would it be ok if I set up a short call to discuss with you? That might help close the gap in my understanding faster. |
|
@hhorak thanks for the full log. My feeling is that the test failure is still and artifact of the initial timeout in the first install. I agree the second npm install should have fixed that up, but its probably more useful to look at the why the timeout occurs if that causes most of the flakiness. In terms of the second install, do our scripts try to do a second install if the first fails or was that just part of your testing. If the scripts do a retry, then it might be possible to add some cleanup so that the second install starts from scratch. |
|
@phracek if you can at mention me in the pino issue once you open it, I'll work to have our team work to move it forward. |
zmiklank
force-pushed
the
unstable_tests
branch
from
e0f06fe
to
721e450
Compare
|
I've divided this PR into two separate ones, leaving here only addition of pino test to the set of unstable tests. |
|
@phracek didyou create an issue in the pino repo? |
|
[test-all] |
|
@mhdawson Not yet. I was trying to do the same steps as @hhorak did, but all were successful. $ podman pull registry.redhat.io/ubi8/nodejs-16
$ podman run -it --rm registry.redhat.io/ubi8/nodejs-16 bash
$ npm show pino version
8.6.1
$ git clone https://github.com/pinojs/pino.git
$ cd pino
$ git checkout v8.4.1
Note: switching to 'v8.4.1'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 7525e3e Bumped v8.4.1
$ npm install
npm WARN deprecated querystring@0.2.0: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.
added 1276 packages, and audited 1439 packages in 3m
152 packages are looking for funding
run `npm fund` for details
8 vulnerabilities (7 moderate, 1 high)
To address issues that do not require attention, run:
npm audit fix
To address all issues (including breaking changes), run:
npm audit fix --force
Run `npm audit` for details.
npm notice
npm notice New minor version of npm available! 8.11.0 -> 8.19.2
npm notice Changelog: https://github.com/npm/cli/releases/tag/v8.19.2
npm notice Run npm install -g npm@8.19.2 to update!
npm notice
$ npm test
> pino@8.4.1 test
> npm run lint && npm run transpile && tap --ts && jest test/jest && npm run test-types
> pino@8.4.1 lint
> eslint .
> pino@8.4.1 transpile
> node ./test/fixtures/ts/transpile.cjs
SKIP test/http.test.js
~ http request support via serializer without request connection
SKIP test/http.test.js 1 skip of 16 183.132ms
~ http request support via serializer without request connection
Suites: 39 passed, 39 of 39 completed
Asserts: 1165 passed, 1 skip, of 1166
----------------------|---------|----------|---------|---------|-------------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
----------------------|---------|----------|---------|---------|-------------------------
All files | 97.56 | 95.77 | 97.58 | 97.68 |
pino | 100 | 100 | 100 | 100 |
browser.js | 100 | 100 | 100 | 100 |
file.js | 100 | 100 | 100 | 100 |
pino.js | 100 | 100 | 100 | 100 |
pino/lib | 96.59 | 93.94 | 96.7 | 96.78 |
caller.js | 86.66 | 50 | 100 | 86.66 | 14,23
levels.js | 100 | 100 | 100 | 100 |
meta.js | 100 | 100 | 100 | 100 |
multistream.js | 98.66 | 97.82 | 100 | 98.63 | 91
proto.js | 100 | 100 | 100 | 100 |
redaction.js | 100 | 100 | 100 | 100 |
symbols.js | 100 | 100 | 100 | 100 |
time.js | 100 | 100 | 100 | 100 |
tools.js | 95.97 | 96 | 88.88 | 95.8 | 221,253-255,289,302,351
transport-stream.js | 52.38 | 30 | 100 | 55.55 | 20-26,32-35
transport.js | 98.33 | 100 | 92.3 | 98.33 | 65
----------------------|---------|----------|---------|---------|-------------------------
PASS test/jest/basic.spec.js
✓ transport should work in jest (11 ms)
Test Suites: 1 passed, 1 total
Tests: 1 passed, 1 total
Snapshots: 0 total
Time: 1.574 s
Ran all test suites matching /test\/jest/i.
> pino@8.4.1 test-types
> tsc && tsd && ts-node test/types/pino.ts
|
|
It looks like pino started to work again with latest version: |
zmiklank
force-pushed
the
unstable_tests
branch
from
721e450
to
841de6b
Compare
|
sclorg/container-common-scripts#268 is merged, so this PR is no longer blocked on it. |
zmiklank
force-pushed
the
unstable_tests
branch
from
43a8ad2
to
8d7bc96
Compare
Adding support for unstable tests.
Depends on sclorg/container-common-scripts#268, thus not yet mergeable or testable in here.