add shutdown that asserts if headscale had panics

[kradalby]

Depends on #2125

Summary by CodeRabbit

• Bug Fixes

  • Enhanced error handling during test shutdown processes to ensure no panics occur, improving test robustness across multiple test files.

• Chores

  • Standardised the shutdown mechanism in tests by replacing scenario.Shutdown() with scenario.ShutdownAssertNoPanics(t) for better error reporting.

[coderabbitai]

Walkthrough

The changes in this pull request focus on enhancing the error handling mechanism across multiple test files in the integration test suite. The primary modification involves replacing calls to scenario.Shutdown() with scenario.ShutdownAssertNoPanics(t), ensuring that any panics during the shutdown process are asserted and reported. This systematic update applies to various test functions, improving the overall robustness of the testing framework.

Changes

Files	Change Summary
integration/acl_test.go, integration/auth_oidc_test.go, integration/auth_web_flow_test.go, integration/cli_test.go, integration/dns_test.go, integration/embedded_derp_test.go, integration/general_test.go, integration/route_test.go, integration/scenario_test.go, integration/ssh_test.go	Replaced defer scenario.Shutdown() with defer scenario.ShutdownAssertNoPanics(t) in multiple test functions to enhance error handling during shutdown.

Possibly related PRs

• make stream shutdown if self-node has been removed #2125: The changes in this PR involve modifying the shutdown mechanism to prevent panics when nodes are deleted, which aligns with the main PR's focus on enhancing error handling during shutdown processes in test cases.

Poem

🐇 In the meadow where bunnies play,
We’ve made our tests safe today!
No more panics, just smooth flows,
With every hop, our confidence grows.
So let’s celebrate, with carrots in hand,
For robust tests, we take a stand! 🌼

---

Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 64319f7 and b9c448f.

Files selected for processing (10)

• integration/acl_test.go (8 hunks)
• integration/auth_oidc_test.go (2 hunks)
• integration/auth_web_flow_test.go (2 hunks)
• integration/cli_test.go (15 hunks)
• integration/dns_test.go (2 hunks)
• integration/embedded_derp_test.go (1 hunks)
• integration/general_test.go (10 hunks)
• integration/route_test.go (5 hunks)
• integration/scenario_test.go (3 hunks)
• integration/ssh_test.go (5 hunks)

Additional comments not posted (54)

integration/scenario_test.go (3)

38-38: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by adding a panic assertion, ensuring that any panics that occur while shutting down the scenario are caught and reported as test failures. This improves the robustness of the tests by providing better error handling and feedback when the shutdown process encounters issues.

---

83-83: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by adding a panic assertion, ensuring that any panics that occur while shutting down the scenario are caught and reported as test failures. This improves the robustness of the tests by providing better error handling and feedback when the shutdown process encounters issues.

---

119-119: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by adding a panic assertion, ensuring that any panics that occur while shutting down the scenario are caught and reported as test failures. This improves the robustness of the tests by providing better error handling and feedback when the shutdown process encounters issues.

integration/embedded_derp_test.go (1)

35-35: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) improves the error handling mechanism by asserting and reporting any panics that occur during the shutdown process. This enhances the robustness and reliability of the test suite, which is consistent with the PR objective and the AI-generated summary.

The code change is approved.

integration/dns_test.go (2)

20-20: LGTM!

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This modification enhances the shutdown process by asserting and reporting any panics that occur during shutdown, leading to more robust error handling in the test.

---

211-211: LGTM!

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This modification is consistent with the change made in TestResolveMagicDNS and enhances the error handling during the shutdown phase by asserting and reporting any panics that occur during shutdown. This contributes to the overall objective of improving the robustness of the test suite.

integration/auth_web_flow_test.go (2)

37-37: LGTM!

The change to call scenario.ShutdownAssertNoPanics(t) instead of scenario.Shutdown() in the defer statement is approved. This update aligns with the renamed method and the additional t *testing.T parameter, which suggests improved error handling during the shutdown process by asserting that no panics occur.

---

76-76: LGTM!

The change to call scenario.ShutdownAssertNoPanics(t) instead of scenario.Shutdown() in the defer statement is approved, similar to the previous change. This update enhances the error handling during the shutdown process by asserting that no panics occur, thereby improving the test robustness.

integration/ssh_test.go (5)

114-114: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) in the defer statement is approved. It aligns with the PR objective of asserting panics during shutdown.

---

179-179: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) in the defer statement is approved. It aligns with the PR objective of asserting panics during shutdown.

---

225-225: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) in the defer statement is approved. It aligns with the PR objective of asserting panics during shutdown.

---

274-274: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) in the defer statement is approved. It aligns with the PR objective of asserting panics during shutdown.

---

330-330: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) in the defer statement is approved. It aligns with the PR objective of asserting panics during shutdown.

integration/auth_oidc_test.go (2)

51-51: LGTM!

The change to assert panics during shutdown is approved. This will improve the robustness of the test by providing better diagnostics in case of failures.

---

111-111: LGTM!

The change to assert panics during shutdown is approved. This will improve the robustness of the test by providing better diagnostics in case of failures.

integration/general_test.go (10)

30-30: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

74-74: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

112-112: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

231-231: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

316-316: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

430-430: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

479-479: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

640-640: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

766-766: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

---

881-881: Improved test shutdown robustness.

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's shutdown sequence by asserting that no panics occur, thereby improving the test's ability to detect and report issues.

integration/acl_test.go (9)

279-279: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

---

319-319: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

---

376-376: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

---

420-420: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

---

476-476: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

---

537-537: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

---

675-675: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

---

Line range hint 1000-1000: Skipped review.

No changes were made to this function.

---

1024-1024: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. This enhances the test's robustness by asserting and reporting any panics during the shutdown process.

integration/route_test.go (5)

35-35: LGTM!

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by asserting that no panics occur, improving the robustness of the test.

---

257-257: LGTM!

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by asserting that no panics occur, improving the robustness of the test.

---

829-829: LGTM!

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by asserting that no panics occur, improving the robustness of the test.

---

971-971: LGTM!

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by asserting that no panics occur, improving the robustness of the test.

---

1062-1062: LGTM!

The change from defer scenario.Shutdown() to defer scenario.ShutdownAssertNoPanics(t) is approved. This enhances the shutdown process by asserting that no panics occur, improving the robustness of the test.

integration/cli_test.go (15)

38-38: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

118-118: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

260-260: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

323-323: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

401-401: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

495-495: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

663-663: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

788-788: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

838-838: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

901-901: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

1142-1142: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

1269-1269: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

1435-1435: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

1596-1596: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

1676-1676: LGTM!

The change from scenario.Shutdown() to scenario.ShutdownAssertNoPanics(t) is approved. It enhances the test's robustness by asserting panics during the shutdown process.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.