Skip to content

enable eslint on wd-test files#6271

Merged
anonrig merged 1 commit intomainfrom
yagiz/enable-eslint-wd-test
Mar 25, 2026
Merged

enable eslint on wd-test files#6271
anonrig merged 1 commit intomainfrom
yagiz/enable-eslint-wd-test

Conversation

@anonrig
Copy link
Copy Markdown
Member

@anonrig anonrig commented Mar 6, 2026

No description provided.

@anonrig anonrig requested review from a team as code owners March 6, 2026 19:46
@anonrig anonrig force-pushed the yagiz/enable-eslint-wd-test branch from 7ad9a2f to c2d0dd1 Compare March 6, 2026 19:47
Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

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

I'm Bonk, and I've done a quick review of your PR.

This PR enables ESLint on .wd-test JS files by integrating lint_test into wd_test.bzl, adding workerd-specific globals, and fixing the resulting lint violations across ~80 files.

  1. [MEDIUM] assert.deepEqual; is a no-op expression (property access, not a function call) -- the eslint-disable suppresses the symptom but leaves a likely pre-existing bug unaddressed.
  2. [LOW] r2-test.js fall-through comments -- the // falls through comments are technically accurate (the code can fall through syntactically) but in practice the inner switch cases always return. Adding break statements would be more correct than endorsing the fall-throughs with comments.

The remaining changes (unused var prefixes, dead code removal, varlet, empty catch/loop bodies, removed stale eslint-disable comments) look correct. The encoding-test.js fix and net-nodejs-test.js rejectthrow fix are genuine bug corrections.


This review was generated by an AI assistant and may contain inaccuracies.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 6, 2026

To https://github.com/cloudflare/workerd.git
! [rejected] yagiz/enable-eslint-wd-test -> yagiz/enable-eslint-wd-test (fetch first)
error: failed to push some refs to 'https://github.com/cloudflare/workerd.git'
hint: Updates were rejected because the remote contains work that you do not
hint: have locally. This is usually caused by another repository pushing to
hint: the same ref. If you want to integrate the remote changes, use
hint: 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 6, 2026

@anonrig Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@anonrig anonrig force-pushed the yagiz/enable-eslint-wd-test branch from c2d0dd1 to 93c279b Compare March 9, 2026 13:01
@anonrig
Copy link
Copy Markdown
Member Author

anonrig commented Mar 9, 2026

Force pushed to fix conflicts & add missing copyright headers to tests

@anonrig anonrig enabled auto-merge March 9, 2026 13:06
@anonrig anonrig force-pushed the yagiz/enable-eslint-wd-test branch 2 times, most recently from 1e8b2a1 to 199c88f Compare March 9, 2026 19:49
@jasnell
Copy link
Copy Markdown
Collaborator

jasnell commented Mar 25, 2026

@anonrig ... what's the status on this one?

@anonrig anonrig force-pushed the yagiz/enable-eslint-wd-test branch from 199c88f to 1b9c849 Compare March 25, 2026 16:08
@anonrig
Copy link
Copy Markdown
Member Author

anonrig commented Mar 25, 2026

@anonrig ... what's the status on this one?

It was blocked on #6397 (due to a conflict with the internal patch). Now that it landed, we can land this as well.

@anonrig anonrig requested a review from jasnell March 25, 2026 16:08
@anonrig anonrig merged commit bcc7a5d into main Mar 25, 2026
22 checks passed
@anonrig anonrig deleted the yagiz/enable-eslint-wd-test branch March 25, 2026 16:36
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