fix(email): unbreak factory_test.go after #333 merge glitch

[cristim]

Summary

• PR fix(email): short-circuit to no-op sender when EMAIL_ENABLED=false (closes #332) #333 (closes fix(email): factory should short-circuit to no-op sender when EMAIL_ENABLED=false #332) merged 25 min ago left internal/email/factory_test.go with two overlapping t.Run("unset_falls_through", ...) blocks — one missing its closing braces, one followed by duplicated ctx := / sender, err := lines. The file no longer parses.
• This trips gofmt + go vet in the pre-commit workflow on every open PR against feat/multicloud-web-frontend (feat(frontend/recs): per-column show/hide with Columns toolbar dropdown #326, fix(local-dev): docker-compose + .env.example cover the new required env vars (closes #334) #335, feat(frontend,email): Archera Insurance CTA in modals + education pages + email mentions (closes #314) #336, …). Independent of any individual PR's changes — the broken bytes live on the base.
• Keep the prev/hadPrev version of the sub-test (the one that actually Unsetenvs EMAIL_ENABLED first, which is what the test name describes), drop the orphaned orig/hadOrig fragment, and remove the duplicate ctx/sender pair.

Reproduction

git checkout origin/feat/multicloud-web-frontend
gofmt -l internal/email/factory_test.go
# internal/email/factory_test.go:309:2: missing ',' before newline in argument list
# internal/email/factory_test.go:312:6: expected '(', found TestNewSenderWithConfig_AWS
# ...

Or check the failing pre-commit step on any open PR against the base — same error shape.

Test plan

• gofmt -l internal/email/factory_test.go — clean
• go vet ./internal/email/... — clean
• go test ./internal/email/... — 306 tests pass
• CI on this PR's pre-commit job goes green (this is the workflow that was previously red on every PR)
• After merge, re-run CI on PR feat(frontend/recs): per-column show/hide with Columns toolbar dropdown #326 / fix(local-dev): docker-compose + .env.example cover the new required env vars (closes #334) #335 / feat(frontend,email): Archera Insurance CTA in modals + education pages + email mentions (closes #314) #336 and confirm the same pre-commit step now passes

Summary by CodeRabbit

• Tests
  • Improved environment variable cleanup and restoration in email factory tests for more robust test isolation.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c68476c6-4cba-4dba-a764-08c93e1b4e1d

📥 Commits

Reviewing files that changed from the base of the PR and between 80ef684 and 3273355.

📒 Files selected for processing (1)

• internal/email/factory_test.go

💤 Files with no reviewable changes (1)

• internal/email/factory_test.go

---

📝 Walkthrough

Walkthrough

The TestNewSenderFromEnvironment_EmailEnabled test's "unset_falls_through" sub-test was adjusted to capture the prior EMAIL_ENABLED environment value using os.LookupEnv and restore or unset it via t.Cleanup, improving test isolation.

Changes

Test Environment Isolation

Layer / File(s)	Summary
Test Environment Capture and Restoration internal/email/factory_test.go	Environment variable handling in the "unset_falls_through" sub-test now snapshots the prior EMAIL_ENABLED value via os.LookupEnv, unsets it for the test, and properly restores or unsets it in t.Cleanup to ensure isolation.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

• LeanerCloud/CUDly#333: Both PRs modify internal/email/factory_test.go and deal with EMAIL_ENABLED test behavior; PR #333 adds the test that PR #337 improves.

Poem

🐰 A test once leaked its env state wild,
Now cleanup captures what ran defiled,
With LookupEnv and Cleanup's care,
The EMAIL_ENABLED flows pure and fair! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	This PR addresses a merge conflict/syntax error in factory_test.go, while linked issue #332 requires functional changes to factory.go and new nop_sender.go. The PR does not implement the coding requirements from #332.	This PR only fixes the test file syntax. Implement the functional requirements from #332: add EMAIL_ENABLED early-return logic, create NopSender implementation, add debug logging, update templates.go logging, and ensure no test regressions.

✅ 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 fix: restoring factory_test.go after a merge conflict from PR #333 caused by overlapping sub-tests and duplicate code.
Out of Scope Changes check	✅ Passed	The PR scope is appropriately limited to restoring the correct test structure in factory_test.go after a merge conflict, with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• ✅ Committed to branch successfully - (🔄 Check to regenerate)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/email-factory-test-merge-glitch

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ 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]

Caution

Docstrings generation - FAILED

No docstrings were generated.