Skip to content

perf(linter/jest-vitest/valid-expect): avoid extra sorting work#23884

Closed
camc314 wants to merge 1 commit into
mainfrom
codex/perf-valid-expect
Closed

perf(linter/jest-vitest/valid-expect): avoid extra sorting work#23884
camc314 wants to merge 1 commit into
mainfrom
codex/perf-valid-expect

Conversation

@camc314

@camc314 camc314 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Split from #23829.

Ports the shared valid-expect collection optimization from #23829 to avoid extra sorting work.

Co-authored-by: Yagiz Nizipli yagiz@nizipli.com

@github-actions github-actions Bot added the A-linter Area - Linter label Jun 28, 2026
@camc314 camc314 self-assigned this Jun 28, 2026
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Jun 28, 2026

camc314 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Jun 28, 11:22 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 28, 11:23 AM UTC: camc314 added this pull request to the Graphite merge queue.
  • Jun 28, 11:28 AM UTC: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (All comments in this PR must be resolved before merging. Once you've resolved all open comment threads, you can retry your merge.).

@camc314 camc314 changed the title perf(linter/jest-vitest): speed up valid-expect perf(linter/jest-vitest/valid-expect): avoid extra sorting work Jun 28, 2026
Split from #23829.

Ports the shared valid-expect collection optimization from #23829 to avoid extra sorting work.

Co-authored-by: Yagiz Nizipli <yagiz@nizipli.com>
@graphite-app graphite-app Bot force-pushed the codex/perf-valid-expect branch from 37d29d3 to fece223 Compare June 28, 2026 11:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37d29d3fea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

possible_jest_nodes.sort_unstable_by_key(|node| node.node.id());
// Order only matters for deterministic diagnostics; node id order is enough
// without an extra sort if we collect in reference walk order (stable enough).
let possible_jest_nodes = collect_possible_jest_call_node(ctx);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve source order before building per-function fixes

When collection order is import/global reference order rather than source order, fixed_function_expression can be set by a later assertion before an earlier one is visited. For example with import { expect as e } from 'vitest'; test('x', () => { expect(p).resolves.toBe(1); e(q).resolves.toBe(2); }), the imported alias is visited first, so its multifix inserts async and spans from the function start to the later call; the earlier global expect then only gets an await fix, but Fixer::fix sorts fixes by span and skips that earlier fix as overlapping. The result of applying suggestions is that the first assertion is still unawaited, which the previous node-id sort avoided.

Useful? React with 👍 / 👎.

@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jun 28, 2026
@camc314 camc314 closed this Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant