Skip to content

fix: use correct GitLab collapsible sections group log format#27

Open
281742759-smi wants to merge 1 commit into
pnpm:mainfrom
281742759-smi:fix/log-group-gitlab
Open

fix: use correct GitLab collapsible sections group log format#27
281742759-smi wants to merge 1 commit into
pnpm:mainfrom
281742759-smi:fix/log-group-gitlab

Conversation

@281742759-smi
Copy link
Copy Markdown

@281742759-smi 281742759-smi commented May 21, 2026

Group logs for GitLab are broken. Node.js doesn't support \e. The updated format follows GitLab's documentation on creating collapsible sections.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed GitLab CI log grouping display to properly render section markers in logs.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

📝 Walkthrough

Walkthrough

This PR updates GitLab CI log grouping escape sequences in log/group/group.ts and corresponding test expectations in log/group/group.spec.ts to use revised ANSI formatting with explicit section level markers and modified carriage-control composition.

Changes

GitLab log grouping

Layer / File(s) Summary
GitLab log grouping escape sequence update
log/group/group.ts, log/group/group.spec.ts
The start and end group label strings now use updated ANSI escape sequence composition (\x1b[0K) with explicit :1 section level markers and revised carriage-return/newline placement. Test assertions were updated to expect the new format including the explicit section markers.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A small hop through escape codes we go,
Where \x1b[0K puts the clearing in show,
Section markers labeled with :1 precision,
Test and code now move in division—
Both changed together, a sync so neat! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: fixing the GitLab collapsible sections group log format to use the correct escape sequences, which directly addresses the issue described in the PR objectives.
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 unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
log/group/group.spec.ts (1)

45-48: 💤 Low value

Hardcoded :1 section id couples this test to module-load order.

The id counter in group.ts is module-scoped and increments on every GitLab groupStart call across the lifetime of the module. Today :1 works because this is the only GitLab test, but adding a second GitLab test case (or reordering) will silently flip id to 2+ and break these assertions. Consider resetting the module (jest.isolateModules / jest.resetModules) before this test, or extracting id into something injectable/testable, so the assertion does not depend on global call ordering.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@log/group/group.spec.ts` around lines 45 - 48, The test hardcodes a GitLab
section id `:1` which relies on the module-scoped counter in group.ts and breaks
when module load/order changes; update the test to avoid coupling to that global
counter by either isolating/resetting the module before the spec (use
jest.isolateModules or jest.resetModules around the test) so the counter starts
from 1, or refactor group.ts to expose/inject the id generator (e.g., accept an
idFactory or resetId function) and in the spec inject or reset it so assertions
on process.stdout.write (around groupStart/groupEnd) do not rely on a global
incrementing value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@log/group/group.spec.ts`:
- Around line 45-48: The test hardcodes a GitLab section id `:1` which relies on
the module-scoped counter in group.ts and breaks when module load/order changes;
update the test to avoid coupling to that global counter by either
isolating/resetting the module before the spec (use jest.isolateModules or
jest.resetModules around the test) so the counter starts from 1, or refactor
group.ts to expose/inject the id generator (e.g., accept an idFactory or resetId
function) and in the spec inject or reset it so assertions on
process.stdout.write (around groupStart/groupEnd) do not rely on a global
incrementing value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f150e90-66e5-4b5c-9373-9592e4c85b8d

📥 Commits

Reviewing files that changed from the base of the PR and between a9b32aa and 1e12303.

📒 Files selected for processing (2)
  • log/group/group.spec.ts
  • log/group/group.ts
📜 Review details
🔇 Additional comments (1)
log/group/group.ts (1)

26-27: LGTM!

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.

1 participant