Skip to content

enable eslint on wd-test files#6271

Open
anonrig wants to merge 1 commit intomainfrom
yagiz/enable-eslint-wd-test
Open

enable eslint on wd-test files#6271
anonrig wants to merge 1 commit intomainfrom
yagiz/enable-eslint-wd-test

Conversation

@anonrig
Copy link
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
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
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
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
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 from 93c279b to 1e8b2a1 Compare March 9, 2026 18:41
@anonrig anonrig force-pushed the yagiz/enable-eslint-wd-test branch from 1e8b2a1 to 199c88f Compare March 9, 2026 19:49
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