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

test: fix flaky timeout in test-pipe-file-to-http #53595

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jakecastelli
Copy link
Contributor

Attempt to fix: #52963

There are a few places could cause the test to timeout, first of all rely on process.exit was probably not a great idea.

Timeout was never cleared, 10mb might be a bit heavy on lower power machine.

Tried another 4000 runs, no failures, should be stable now?

Will actively monitor the issue once this PR lands to see if there is still any failure in CI.

Previous attempt PR was closed and cannot re-opened.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jun 26, 2024
@jakecastelli jakecastelli changed the title test: fix flaky timeout test: fix flaky timeout in test-pipe-file-to-http Jun 26, 2024
@lpinca
Copy link
Member

lpinca commented Jun 27, 2024

Are you still able to get timeouts on main without this patch?

@jakecastelli
Copy link
Contributor Author

Yes, I can still get timeout on main - it is reproducible in every 1000 run.

@lpinca
Copy link
Member

lpinca commented Jun 27, 2024

Yes, I can still get timeout on main - it is reproducible in every 1000 run.

When the test times out, is the 'end' event emitted on the server response? You can check that by adding something like this:

res.on('end',  () => {
  console.log('end');
  server.close();
});

@jakecastelli
Copy link
Contributor Author

Can I ask how to ask the ./tools/test.py to print console.logs, I tried with -v flag ./tools/test.py --repeat=1000 -v test/parallel/test-pipe-file-to-http.js but I still cannot see the console output from console.log

@lpinca
Copy link
Member

lpinca commented Jun 27, 2024

It will do it automatically for failed tests. You should see something like "TIMEOUT" and then the console.log() stuff.

@jakecastelli
Copy link
Contributor Author

end is emitted

Path: parallel/test-pipe-file-to-http
end
Command: out/Release/node node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---

@lpinca
Copy link
Member

lpinca commented Jun 27, 2024

Ok, then it is not related to the file size either. The process for some reason (#52964 (comment), #52959 (comment)) does not exit.

@jakecastelli
Copy link
Contributor Author

I tried a few more runs - sometimes the end is not printed though

my code snippet for conosle log
    res.on('end', () => {
      console.log('end called');

      assert.strictEqual(count, fileSize);

      console.log('closing the server');

      server.close();

      console.log('server should be closed')
    });
Error logs
=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Command: out/Release/node ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---


=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
end called
closing the server
server should be closed
Command: out/Release/node./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---

=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
end called
closing the server
server should be closed
Command: out/Release/node ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---


=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Command: out/Release/node ./node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---

I have also tried the --jitless flag again, and I found interesting when the test is timeout it says Warning: disabling flag --expose_wasm due to conflicting flags but I have not used --expose_wasm anywhere.

=== release test-pipe-file-to-http ===                    
Path: parallel/test-pipe-file-to-http
Warning: disabling flag --expose_wasm due to conflicting flags
Command: out/Release/node --jitless ./oss/node/test/parallel/test-pipe-file-to-http.js
--- TIMEOUT ---

@jakecastelli
Copy link
Contributor Author

Do you have any idea why Warning: disabling flag --expose_wasm due to conflicting flags showed up when I only used --jitless 👀

@lpinca
Copy link
Member

lpinca commented Jun 29, 2024

No, I don't know.

@jakecastelli
Copy link
Contributor Author

Some update:

Been running this test on a Linux ubuntu x64 machine, more than 100k+ runs, no failure.

But running on my m1 arm64 will have a quite consistent 0.1% failure rate. With --jitless I would see the Warning: disabling flag --expose_wasm due to conflicting flags when test timeout, not sure if it is related, the closest thing I can find is this, maybe the failure is related to arm build only?

May I ask what environment you are using @lpinca?

Will be looking into this for another day, I think we can probably add this test to the flaky test status until we can find more clues. Any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test-pipe-file-to-http is flaky (timeout)
3 participants