Skip to content

test: enable 2 already-passing Node compat tests#33706

Merged
bartlomieju merged 1 commit intodenoland:mainfrom
nathanwhitbot:fix/node-compat-iter72
Apr 29, 2026
Merged

test: enable 2 already-passing Node compat tests#33706
bartlomieju merged 1 commit intodenoland:mainfrom
nathanwhitbot:fix/node-compat-iter72

Conversation

@nathanwhitbot
Copy link
Copy Markdown
Contributor

Summary

Enables 2 Node.js compat tests that already pass on main without any code changes. No production code touched — just tests/node_compat/config.jsonc entries.

  • parallel/test-tls-junk-closes-server.js
  • parallel/test-tls-no-rsa-key.js

Test plan

  • Each test verified passing 3/3 times via the node_compat test runner
  • Full cargo test --test node_compat run on Linux confirms no regressions (only pre-existing flaky failures)

@nathanwhitbot nathanwhitbot force-pushed the fix/node-compat-iter72 branch from ad0230e to aaecedd Compare April 29, 2026 15:08
@nathanwhitbot
Copy link
Copy Markdown
Contributor Author

Force-pushed aaeceddb3 — branch was stacked on #33653 as a base. Reset to upstream/main and cherry-picked just the 2-test commit. Diff is now 1 file, +2 -0.

Copy link
Copy Markdown
Contributor Author

@nathanwhitbot nathanwhitbot left a comment

Choose a reason for hiding this comment

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

Self-review of `aaeceddb3` (post-cleanup):

Pure config-only test enrollment, no production code. Both entries alphabetically positioned correctly:

Test Prev Next
`test-tls-junk-closes-server.js` `test-tls-js-stream.js` `test-tls-keyengine-invalid-arg-type.js`
`test-tls-no-rsa-key.js` `test-tls-no-cert-required.js` `test-tls-no-sslv3.js`

Both enabled with empty config (`{}`). Holding to COMMENT until CI confirms.

Copy link
Copy Markdown
Contributor Author

@nathanwhitbot nathanwhitbot left a comment

Choose a reason for hiding this comment

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

Single-commit, 2-line config-only enrollment. No scope-creep. LGTM — TLS tests can be platform-sensitive, so if either comes back red on Windows or macOS drop rather than gate.

@nathanwhitbot
Copy link
Copy Markdown
Contributor Author

test integration (1/2) debug macos-aarch64 fail is DNS resolution flake (nodename nor servname provided) — pure GH macOS aarch64 infra. @nathanwhit re-run when convenient.

Copy link
Copy Markdown
Contributor

@fibibot fibibot left a comment

Choose a reason for hiding this comment

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

LGTM. Pure config-only enrollment, 2 entries, both alphabetically positioned correctly:

  • test-tls-junk-closes-server.js between test-tls-js-stream and test-tls-keyengine-invalid-arg-type (j-s < j-u-nk < k). ✓
  • test-tls-no-rsa-key.js between test-tls-no-cert-required and test-tls-no-sslv3 (no-c < no-r < no-s). ✓

CI: 128 success, 1 substantive failure, 3 still pending. The single failure (test integration (1/2) debug macos-aarch64) is a rustup-channel-download DNS error during build setup:

error: could not download file from 'https://static.rust-lang.org/dist/channel-rust-1.92.0.toml.sha256'
  3: dns error
  4: failed to lookup address information: nodename nor servname provided, or not known

The shard never reached the test phase, and a config-only PR can't cause a DNS lookup to fail. Pure infra flake.

@bartlomieju bartlomieju enabled auto-merge (squash) April 29, 2026 16:06
@bartlomieju bartlomieju merged commit b48ad75 into denoland:main Apr 29, 2026
268 of 270 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants