iframe-proxy

cristim · 2026-04-28T00:13:03Z

Summary

Continues the t.Parallel() adoption sweep from #58. This PR targets pkg/config/ and uses the standard Go testing mixed-mode pattern: 10 pure tests are parallelised, while 8 tests that mutate process-wide state (t.Setenv / os.Chdir) intentionally remain serial — Go's t.Setenv panics if combined with t.Parallel(), so making them parallel is not a "fix" but a regression.

To prevent future contributors from accidentally adding t.Parallel() to the serial tests, each one carries an inline // NOT parallel: ... comment explaining why.

Per-test classification

Parallel (10) — pure, no env/chdir/global mutation:

TestLoad_Defaults
TestLoad_ExplicitMissingFile_Error
TestLoad_YAMLFile
TestLoad_InvalidYAML_Error
TestLoad_DryRunAndPurchaseConflict
TestLoad_PurchaseFlagSetsDryRunFalse
TestLoad_UnknownCloud_Error
TestLoad_NegativeThreshold_Error
TestValidate_EmptyEnabledClouds_Error
TestLoad_YAMLRoundtrip

Serial (8) — documented in-source:

TestLoad_MissingDefaultFile_NoError — os.Chdir
TestLoad_EnvOverridesYAML — t.Setenv
TestLoad_EnvVars — t.Setenv (×11)
TestLoad_InvalidEnvVar_Error — t.Setenv
TestLoad_InvalidDurationEnvVar_Error — t.Setenv
TestLoad_FlagOverridesEnvAndYAML — t.Setenv
TestLoad_CUDLYCONFIGEnv — t.Setenv
TestLoad_ArgPathTakesPrecedenceOverCUDLYCONFIG — t.Setenv

Out of scope

internal/config/ (Postgres integration tests — explicitly deferred per chore(tests): finish t.Parallel() adoption sweep across remaining packages #58 body, needs a separate refactor).
Refactoring t.Setenv users to a different env-injection pattern — would change the test design rather than just parallelise.

Test plan

cd pkg && go test -race -count=2 ./config/... passes cleanly twice locally
All pre-commit hooks pass (gosec, trivy, complexity, full Go tests)
CI green on PR

Refs #58

Summary by CodeRabbit

Chores
- Internal test infrastructure optimizations with no impact on user-facing functionality.

coderabbitai · 2026-04-28T00:13:10Z

Warning

Rate limit exceeded

@cristim has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 8 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 19615275-c620-4ac3-ae53-05056a93d0b2

📥 Commits

Reviewing files that changed from the base of the PR and between 1dd7b9a and e963008.

📒 Files selected for processing (1)

pkg/config/config_test.go

📝 Walkthrough

Walkthrough

This PR adds test concurrency annotations to pkg/config/config_test.go. Most TestLoad_* tests and TestValidate_EmptyEnabledClouds_Error now call t.Parallel() for concurrent execution. Tests using process-wide mutations like os.Chdir and t.Setenv remain sequential with documentation comments explaining the constraint.

Changes

Cohort / File(s)	Summary
Test Concurrency Optimization `pkg/config/config_test.go`	Added `t.Parallel()` calls to most test functions to enable concurrent test execution. Tests with process-wide dependencies (`os.Chdir`, `t.Setenv`) marked sequential with explanatory comments.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

chore(tests): finish t.Parallel() adoption sweep across remaining packages #58: This PR directly implements part of the codebase-wide effort to adopt t.Parallel() across tests, with explicit attention to avoiding parallelization in tests that modify process-wide state (e.g., via os.Setenv), which is exactly the constraint documented in issue #58.

Poem

🐰 Hop hop, the tests now race,
Parallel paths at perfect pace,
Yet some must wait their solo turn,
When shared state makes us all concerned!
Faster CI, cleaner code divine! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: parallelizing tests in pkg/config while noting that t.Setenv users remain serial. It is concise, specific, and directly related to the changeset.
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
Commit unit tests in branch chore/tparallel-pkg-config

_{Review rate limit: 0/5 reviews remaining, refill in 3 minutes and 8 seconds.}

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

cristim · 2026-04-28T00:13:11Z

@coderabbitai review

coderabbitai · 2026-04-28T00:13:16Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-04-28T00:20:34Z

@coderabbitai review

coderabbitai · 2026-04-28T00:20:39Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

🧹 Nitpick comments (1)

pkg/config/config_test.go (1)

58-60: Handle working-directory setup/teardown errors explicitly.

Line 58 and Line 60 currently ignore errors; if either fails, the test can leak state or mask root cause.

Suggested hardening

-	orig, _ := os.Getwd()
+	orig, err := os.Getwd()
+	require.NoError(t, err)
 	require.NoError(t, os.Chdir(dir))
-	t.Cleanup(func() { os.Chdir(orig) })
+	t.Cleanup(func() { require.NoError(t, os.Chdir(orig)) })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/config/config_test.go` around lines 58 - 60, The test currently ignores
errors from os.Getwd() and the cleanup os.Chdir(orig); update the setup to check
and fail the test on error (use require.NoError or t.Fatalf) for os.Getwd() and
the initial os.Chdir(dir), and change the t.Cleanup closure to capture and
handle the error returned by os.Chdir(orig) (call require.NoError or log/fatal
inside the closure) so any failure to restore the working directory is surfaced;
reference the os.Getwd, os.Chdir and t.Cleanup calls in
pkg/config/config_test.go when making these changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/config/config_test.go`:
- Around line 58-60: The test currently ignores errors from os.Getwd() and the
cleanup os.Chdir(orig); update the setup to check and fail the test on error
(use require.NoError or t.Fatalf) for os.Getwd() and the initial os.Chdir(dir),
and change the t.Cleanup closure to capture and handle the error returned by
os.Chdir(orig) (call require.NoError or log/fatal inside the closure) so any
failure to restore the working directory is surfaced; reference the os.Getwd,
os.Chdir and t.Cleanup calls in pkg/config/config_test.go when making these
changes.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ebd45a01-ad3d-4d0c-aed3-b9a00c0d8cda

📥 Commits

Reviewing files that changed from the base of the PR and between 3e655cb and 1dd7b9a.

📒 Files selected for processing (1)

pkg/config/config_test.go

Per CodeRabbit nitpick on PR #175: TestLoad_MissingDefaultFile_NoError ignored errors from os.Getwd() and the t.Cleanup os.Chdir(orig). If either fails the test could leak working-directory state into other tests or mask the root cause of a failure. Wrap both in require.NoError.

…stay serial) Per-test classification (10 parallel + 8 documented-serial): Parallel (10) — pure, no env/chdir/global mutation: - TestLoad_Defaults - TestLoad_ExplicitMissingFile_Error - TestLoad_YAMLFile - TestLoad_InvalidYAML_Error - TestLoad_DryRunAndPurchaseConflict - TestLoad_PurchaseFlagSetsDryRunFalse - TestLoad_UnknownCloud_Error - TestLoad_NegativeThreshold_Error - TestValidate_EmptyEnabledClouds_Error - TestLoad_YAMLRoundtrip Serial (8) — Go's t.Setenv / os.Chdir mutate process-wide state and panic if combined with t.Parallel(). Each gets an inline NOT-parallel comment so future contributors don't accidentally add t.Parallel(): - TestLoad_MissingDefaultFile_NoError (os.Chdir) - TestLoad_EnvOverridesYAML (t.Setenv) - TestLoad_EnvVars (t.Setenv x11) - TestLoad_InvalidEnvVar_Error (t.Setenv) - TestLoad_InvalidDurationEnvVar_Error (t.Setenv) - TestLoad_FlagOverridesEnvAndYAML (t.Setenv) - TestLoad_CUDLYCONFIGEnv (t.Setenv) - TestLoad_ArgPathTakesPrecedenceOverCUDLYCONFIG (t.Setenv) Verified: `go test -race -count=2 ./config/...` (from pkg/) passes cleanly twice. Refs #58

Per CodeRabbit nitpick on PR #175: TestLoad_MissingDefaultFile_NoError ignored errors from os.Getwd() and the t.Cleanup os.Chdir(orig). If either fails the test could leak working-directory state into other tests or mask the root cause of a failure. Wrap both in require.NoError.

cristim · 2026-04-29T22:50:37Z

Rebased onto current feat/multicloud-web-frontend to bring CR review current. @coderabbitai review

coderabbitai · 2026-04-29T22:50:48Z

coderabbitai Bot reviewed Apr 28, 2026

View reviewed changes

cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/eventually No deadline impact/internal Team-internal only effort/xs Trivial / one-liner type/chore Maintenance / non-user-visible labels Apr 28, 2026

cristim added 2 commits April 30, 2026 00:49

cristim force-pushed the chore/tparallel-pkg-config branch from e33cbcf to e963008 Compare April 29, 2026 22:50

cristim merged commit c13433d into feat/multicloud-web-frontend May 6, 2026
4 checks passed

cristim deleted the chore/tparallel-pkg-config branch May 6, 2026 13:48

cristim mentioned this pull request May 28, 2026

chore(tests): finish t.Parallel() adoption sweep across remaining packages #58

Closed

cristim mentioned this pull request Oct 7, 2026

fix(recommendations): use pool demand for expiry coverage #2133

Merged

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

chore(tests): parallelize pkg/config tests (partial — t.Setenv users stay serial) - #175

chore(tests): parallelize pkg/config tests (partial — t.Setenv users stay serial)#175
cristim merged 2 commits into
feat/multicloud-web-frontendfrom
chore/tparallel-pkg-config

cristim commented Apr 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 28, 2026 •

edited

Loading

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

cristim commented Apr 28, 2026

Uh oh!

coderabbitai Bot commented Apr 28, 2026

Uh oh!

cristim commented Apr 28, 2026

Uh oh!

coderabbitai Bot commented Apr 28, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

cristim commented Apr 29, 2026

Uh oh!

coderabbitai Bot commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

cristim commented Apr 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Per-test classification

Out of scope

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Poem

❌ Failed checks (1 warning)

Uh oh!

cristim commented Apr 28, 2026

Uh oh!

coderabbitai Bot commented Apr 28, 2026

Uh oh!

cristim commented Apr 28, 2026

Uh oh!

coderabbitai Bot commented Apr 28, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cristim commented Apr 29, 2026

Uh oh!

coderabbitai Bot commented Apr 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cristim commented Apr 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 28, 2026 •

edited

Loading