[jsweep] Clean action_setup_otlp.cjs

[github-actions]

Summary

Cleaned action_setup_otlp.cjs — a Node.js context file (not github-script).

Changes

action_setup_otlp.cjs

• Simplified SETUP_START_MS parsing: replaced parseInt(..., 10) + Number.isFinite with the cleaner Number() + Number.isFinite pattern — removes the unnecessary || "0" fallback and is more semantically precise (rejects partial strings like "123abc" instead of parsing 123)

action_setup_otlp.test.cjs

• Added test: "should pass startMs=0 when SETUP_START_MS is a partial number string like '123abc'" — documents the new strict parsing behavior (was previously silently parsing 123 via parseInt)

Test improvements

• Before: 35 tests
• After: 36 tests (+1)

Context

• Type: Node.js context (no github/core globals)
• No @ts-nocheck: file already had @ts-check enabled

Validation ✅

• Formatting: npm run format:cjs ✓
• Linting: npm run lint:cjs ✓
• Type checking: npm run typecheck ✓
• Tests: all 36 tests in action_setup_otlp.test.cjs pass ✓

Generated by jsweep - JavaScript Unbloater · ● 1.1M · ◷

• expires on Apr 29, 2026, 4:52 AM UTC

[github-actions]

Hey @github-actions[bot] 👋 — thanks for this clean-up of action_setup_otlp.cjs! The change is focused, well-described, and includes a new test covering the stricter Number() parsing behaviour. The PR looks technically solid.

However, this repository does not have a CONTRIBUTING.md, so there are no guidelines to evaluate against — the ❓ verdict reflects that gap rather than any issue with the PR itself. Maintainers may want to add contribution guidelines to help future contributors and automated tooling alike.

Generated by Contribution Check · ● 1.4M · ◷

[Copilot]

Pull request overview

Cleans up action_setup_otlp.cjs by adjusting how SETUP_START_MS is parsed and adds a regression test to document the stricter behavior for partial numeric strings.

Changes:

• Updated SETUP_START_MS parsing to use Number() + Number.isFinite and default to 0 when invalid.
• Added a test case asserting that partial numeric strings like "123abc" result in startMs=0.

Show a summary per file

File	Description
actions/setup/js/action_setup_otlp.cjs	Changes SETUP_START_MS parsing logic before passing startMs into sendJobSetupSpan.
actions/setup/js/action_setup_otlp.test.cjs	Adds coverage for partial-number SETUP_START_MS input ("123abc").

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 1

[Copilot]

Number(process.env.SETUP_START_MS) changes the accepted formats compared to the previous parseInt(..., 10): it will treat strings like "1e3" as 1000 and "0x10" as 16 (and it will accept decimals), which can silently produce an unexpected epoch-ms value. If SETUP_START_MS is intended to be a base-10 integer timestamp, consider validating the env var as a decimal-integer string (optionally trimmed) and only then converting (e.g., also requiring Number.isSafeInteger), otherwise fall back to 0.

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

✅ Excellent

Metric	Value
New/modified tests analyzed	1
✅ Design tests (behavioral contracts)	1 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	1 (100%)
Duplicate test clusters	0
Test inflation detected	Yes (8 test lines / 2 prod lines = 4:1)
🚨 Coding-guideline violations	None

---

Test Classification Details

Test	File	Classification	Issues Detected
should pass startMs=0 when SETUP_START_MS is a partial number string like '123abc'	actions/setup/js/action_setup_otlp.test.cjs	✅ Design	Minor: test inflation ratio (4:1), but test is well-justified

---

Flagged Tests — Requires Review

No tests require mandatory review. One minor note:

i️ Test Inflation Ratio (informational, not blocking)

The test file adds 8 lines vs. 2 production lines (4:1 ratio), which technically exceeds the 2:1 threshold. However, this is contextually reasonable: the single new test verifies a subtle but important behavioral difference between parseInt("123abc", 10) (→ 123) and Number("123abc") (→ NaN). The test inflation metric deducted 10 points but does not indicate low-value testing here.

---

Language Support

Tests analyzed:

• 🟨 JavaScript (*.test.cjs): 1 test (vitest)

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). The new test directly enforces the behavioral contract of the SETUP_START_MS parsing refactor, covering the key edge case ("123abc" partial number string) that motivates the change from parseInt to Number.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §24998136103

🧪 Test quality analysis by Test Quality Sentinel · ● 379.8K · ◷

[github-actions]

✅ Test Quality Sentinel: 80/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). The new test covers the key edge case for the parseInt → Number refactor.

[pelikhan]

@copilot review all comments

[Copilot]

@copilot review all comments

Addressed in c584709. The parsing now uses /^\d+$/ to validate the env var is a plain decimal-integer string before converting, and then additionally checks Number.isSafeInteger. This rejects scientific notation ("1e3" → 0), hex ("0x10" → 0), decimals ("1.5" → 0), negatives, and partial strings. Two new tests document these edge cases (38 total, all passing).