Skip to content

test(app): normalize perf workflow line endings#623

Merged
Astro-Han merged 1 commit into
devfrom
codex/fix-windows-perf-workflow-eol
May 14, 2026
Merged

test(app): normalize perf workflow line endings#623
Astro-Han merged 1 commit into
devfrom
codex/fix-windows-perf-workflow-eol

Conversation

@Astro-Han
Copy link
Copy Markdown
Owner

Summary

Normalize line endings before asserting snippets from .github/workflows/perf-probe-baseline.yml in the app perf workflow contract test.

Add a focused CRLF regression case so Windows checkout behavior stays covered.

Why

Windows advisory has been failing in unit-windows-app since PR #610 because packages/app/src/testing/perf-workflow.test.ts reads a workflow file and matches a literal \n snippet. On Windows checkout, the workflow content is read as \r\n, so the test fails even though the workflow content is correct.

This is a test portability fix, not a product runtime change.

Related Issue

None. This fixes the current Windows advisory failure introduced by the perf workflow contract test.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

  • packages/app/src/testing/perf-workflow.test.ts: line-ending normalization is scoped to workflow text read by this test.
  • The CRLF regression case matches the Windows advisory failure without changing workflow behavior.

Risk Notes

Low. Test-only change. No runtime, packaging, updater, or workflow execution behavior changed.

How To Verify

Red check: added CRLF regression case first and confirmed the old assertion failed.
Focused test: bun test --preload ./happydom.ts ./src/testing/perf-workflow.test.ts -> 2 passed, 0 failed.
App unit CI parity: bun run test:ci in packages/app -> 1067 passed, 0 failed.
Diff check: git diff --check -> clean.

Screenshots or Recordings

None. No visible UI change.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has type, primary area, and priority labels, or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

@github-actions github-actions Bot added the app Application behavior and product flows label May 14, 2026
@Astro-Han Astro-Han added ci Continuous integration / GitHub Actions P2 Medium priority windows Windows-specific labels May 14, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Suggested priority: P2 (includes user-path files (packages/app/src/testing/perf-workflow.test.ts)).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@github-actions
Copy link
Copy Markdown

Perf delta summary

Comparator: pass

Profile / Scenario interaction median interaction worst long task max tbt frame gap p95 frame gap max jank count cls status
default / homepage-cold 56 -> 24 (-32) 64 -> 64 (0) 82 -> 66 (-16) 32 -> 16 (-16) 33.3 -> 33.4 (+0.1) 150 -> 116.6 (-33.4) 5 -> 3 (-2) 0 -> 0 (0) pass
default / session-streaming-long 56 -> 48 (-8) 64 -> 64 (0) 0 -> 0 (0) 0 -> 0 (0) 33.4 -> 16.8 (-16.6) 66.6 -> 33.3 (-33.3) 1 -> 0 (-1) 0 -> 0 (0) pass
default / tool-call-expand 16 -> 16 (0) 16 -> 16 (0) 0 -> 0 (0) 0 -> 0 (0) 16.7 -> 16.8 (+0.1) 16.7 -> 16.8 (+0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / tool-default-open-heavy-bash 24 -> 24 (0) 24 -> 24 (0) 65 -> 66 (+1) 15 -> 16 (+1) 50 -> 49.9 (-0.1) 100 -> 150 (+50) 3 -> 2 (-1) 0 -> 0 (0) pass
default / terminal-side-panel-open 40 -> 48 (+8) 48 -> 48 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.7 (-0.1) 16.8 -> 16.7 (-0.1) 0 -> 0 (0) 0 -> 0 (0) pass
default / session-scroll-reading 24 -> 32 (+8) 32 -> 32 (0) 0 -> 0 (0) 0 -> 0 (0) 16.8 -> 16.7 (-0.1) 16.8 -> 16.7 (-0.1) 0 -> 0 (0) 0.505 -> 0.505 (0) warn: cls

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8acf5ba1-2757-4a45-b7b9-11ef2a0b1105

📥 Commits

Reviewing files that changed from the base of the PR and between 7e53c57 and 67975ae.

📒 Files selected for processing (1)
  • packages/app/src/testing/perf-workflow.test.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: unit-windows-opencode-config-project
  • GitHub Check: unit-windows-app
  • GitHub Check: unit-windows-opencode-session
  • GitHub Check: unit-windows-opencode-server-tools
  • GitHub Check: unit-windows-desktop
🧰 Additional context used
📓 Path-based instructions (1)
packages/app/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/app/AGENTS.md)

Always prefer createStore over multiple createSignal calls in SolidJS

Files:

  • packages/app/src/testing/perf-workflow.test.ts
🧠 Learnings (2)
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.

Applied to files:

  • packages/app/src/testing/perf-workflow.test.ts
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.

Applied to files:

  • packages/app/src/testing/perf-workflow.test.ts
🔇 Additional comments (1)
packages/app/src/testing/perf-workflow.test.ts (1)

4-5: LGTM!

Also applies to: 7-12, 14-16


📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • Tests
    • Enhanced test infrastructure to properly handle different line ending formats (Windows CRLF vs Unix LF) in workflow validation tests, ensuring consistent test results across platforms.

Walkthrough

This PR adds a normalizeLineEndings helper function and applies it to performance workflow tests to handle cross-platform line-ending differences. The baseline workflow contract test now normalizes CRLF to LF before asserting expected steps, restore keys, and artifact/comment file references.

Changes

Cross-platform workflow test normalization

Layer / File(s) Summary
Line-ending normalization helper
packages/app/src/testing/perf-workflow.test.ts
normalizeLineEndings(text: string) replaces CRLF and CR sequences with \n to normalize Windows and legacy line endings for test assertions.
Workflow contract test updates
packages/app/src/testing/perf-workflow.test.ts
Adds a new CRLF normalization test case and updates the existing perf-probe-baseline workflow contract test to normalize loaded YAML content before checking for expected steps, restore-keys content, and artifact/comment file references.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • Astro-Han/pawwork#509: Both PRs adjust Windows-specific test assertions to tolerate CRLF vs LF line endings via helper functions or regex patterns.
  • Astro-Han/pawwork#429: Both PRs update test assertions to be Windows-robust by normalizing CRLF→LF line endings before comparing fixture/YAML content.

Poem

🐰 On Windows, line endings cause a fuss,

CRLF and LF both travel the test bus,

Now normalized helpers make assertions bright,

Cross-platform checks work left and right! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: normalizing line endings in perf workflow tests, which directly matches the core objective.
Description check ✅ Passed The description covers all required sections with substantial detail: Summary, Why, Related Issue, Human Review Status, Review Focus, Risk Notes, How To Verify, Screenshots/Recordings, and a completed checklist.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-windows-perf-workflow-eol

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a normalizeLineEndings utility to the performance workflow tests to handle cross-platform line-ending inconsistencies. It includes a new test case to verify the normalization logic and applies it to the existing workflow contract test. I have no feedback to provide as no review comments were submitted.

@Astro-Han Astro-Han merged commit 4401172 into dev May 14, 2026
32 checks passed
@Astro-Han Astro-Han deleted the codex/fix-windows-perf-workflow-eol branch May 14, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app Application behavior and product flows ci Continuous integration / GitHub Actions P2 Medium priority windows Windows-specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant