test: deflake sequential/test-tls-session-timeout

[joyeecheung]

This patch:

• Splits the validation tests into a separate file and keep the
  test focus on functional test of the sessionTimeout option.
• Increase the testing timeout to 5 seconds in case it takes too
  long for the first connection to complete and the session is
  already expired when the second connection is started.
• Use a specific sessionIdContext to ensure stable session ID.
• Fix the s_client arguments by specifying CA file and server name.
• Do not use the serialized session ticket for the first connection.
  That was genearted years ago and may not work in different OpenSSL
  versions. Let the first fresh connection generate the ticket.
• Use random port instead of the common port.
• Add a timeout before the second connection to ensure session ticket
  is properly written.
• Log information to faciliate debugging.

Fixes: #26839

---

I have no idea if this actually fixes the test because I can't reproduce it locally. I just noticed that it's missing the -reconnect option which is theoratically necessary if you actually want session reuse and I figured adding it back might do something. This test has been failing pretty badly (failing 28 PRs out of the last 100 CI runs! nodejs/reliability#1282) so any attempt seems useful ¯\(ツ)/¯. Another idea is probably adding a setTimeout after the first connection.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68525/

[joyeecheung]

Actually I realized that -reconnect is probably changing the purpose of the test. Switched to setTimeout and at least log more information out of the test..

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68526/

[joyeecheung]

Stress test on Windows and RHEL8-x64: https://ci.nodejs.org/job/node-stress-single-test/600/
Stress test on main: https://ci.nodejs.org/job/node-stress-single-test/601/

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.90%. Comparing base (a73b575) to head (8458534).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #59423      +/-   ##
==========================================
- Coverage   89.91%   89.90%   -0.02%     
==========================================
  Files         655      655              
  Lines      192866   192866              
  Branches    37806    37805       -1     
==========================================
- Hits       173412   173387      -25     
- Misses      12015    12038      +23     
- Partials     7439     7441       +2     

see 50 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[joyeecheung]

It's hard to understand what this test tries to do now but I suppose the failure might have something to do with:

Verification error: self-signed certificate

?

[joyeecheung]

My new theory: the verification error makes s_client less likely to reuse the session. Fixed the verification by pointing s_client to the CA file. Let's if it does anything..

New stress test: https://ci.nodejs.org/job/node-stress-single-test/602/

[joyeecheung]

Two more ideas:

1. Stack trace is showing Can't use SSL_get_servername. That might affect the session ticket stability? Anyway, no harm in explicitly specifying -servername localhost
2. It might need a stable sessionIdContext for consistent session IDs. Added one.

[joyeecheung]

New stress test: https://ci.nodejs.org/job/node-stress-single-test/603/

[joyeecheung]

Two new ideas:

1. 1 second of session timeout may be too small and the session already expired by the time the second attempt is started. Increase it to 5 seconds
2. Maybe the first connection does not need to use the session ticket checked into the source, or there can be some version mismatch. Just serialize for the first connection.

The comment

  // We need to store a sample session ticket in the fixtures directory because
  // `s_client` behaves incorrectly if we do not pass in both the `-sess_in`
  // and the `-sess_out` flags, and the `-sess_in` argument must point to a
  // file containing a proper serialization of a session ticket.
  // To avoid a source control diff, we copy the ticket to a temporary file.

is from 12 years ago, it may no longer be true for newer OpenSSL.

Stress test: https://ci.nodejs.org/job/node-stress-single-test/604/

[joyeecheung]

Looks like the changes fix the flake under the selected stress test. Squashed them and amended the commit message.

New stress test: https://ci.nodejs.org/job/node-stress-single-test/605/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68537/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68538/

[JakobJingleheimer]

🙌

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/68564/

[nodejs-github-bot]

Landed in f993fca