chore(app): retaining sync start errs#75
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes error handling during the startup phase for both the App and the ServiceEndpointHashBucket by replacing local startErr variables with struct-level fields, ensuring error states are tracked consistently.
- Replaced local error variables with struct-level startErr fields in both App and ServiceEndpointHashBucket.
- Updated error assignments and returns in the Start methods for consistent handling.
- Modified the CI workflow to enable test shuffling with the -shuffle=on flag.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cache/service_endpoint_hash_bucket.go | Centralized error handling by replacing local startErr with a struct-field. |
| app.go | Refactored startup error handling to use the struct-level startErr field. |
| .github/workflows/ci-code-approval.yml | Enabled test shuffling by adding the -shuffle=on flag to go test. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors startup error handling by replacing local error variables with struct-level fields and updates test timing verification accordingly. In addition, it updates several tests for better parallel execution and introduces shuffling in CI test runs.
- Centralized error handling in App and ServiceEndpointHashBucket by storing startup errors in struct-level fields.
- Removed the global timestamp variable in favor of direct time.Now().UTC() calls and updated tests to use WithinDuration for time comparisons.
- Improved test robustness with parallel subtests and enabled test shuffling in CI.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| health/time.go | Removed the timestamp variable to rely on direct time retrieval. |
| health/status.go | Added UnmarshalJSON implementation for Status with explicit errors. |
| health/state_test.go | Updated time comparisons from strict equality to WithinDuration. |
| health/result_test.go | Adjusted JSON decoding tests to use fewer hardcoded time values. |
| health/checker_test.go | Refactored subtests & replaced timestamp mocks; introduced time.Sleep |
| health/checker.go | Replaced timestamp() with time.Now().UTC() for consistency. |
| health/check_test.go | Converted legacy tests to use subtests and WithinDuration comparisons. |
| cache/service_endpoint_hash_bucket.go | Replaced local startErr with the struct field for consistent error handling. |
| app.go | Updated Start method to use the struct-level startErr field. |
| .github/workflows/ci-code-approval.yml | Enabled test shuffling (-shuffle=on) during CI runs. |
Comments suppressed due to low confidence (2)
health/checker_test.go:326
- Sleeping in tests can increase execution time and may lead to flakiness. Consider using a time mocking approach to simulate time passage in error grace period tests.
time.Sleep(3 * time.Second) // Sleep for 3 seconds to exceed the grace period
health/checker_test.go:372
- Using sleep in tests may slow down execution and lead to nondeterministic timing issues. Consider replacing the sleep with a time simulation or mocking mechanism to reliably exceed the grace period.
time.Sleep(3 * time.Second) // Sleep for 3 seconds to exceed the grace period
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Describe your changes
This pull request introduces improvements to error handling, test structure, and code reliability across multiple files. Key changes include replacing local
startErrvariables with struct-level fields for better error tracking, refactoring health check tests for parallel execution and clarity, and updating timestamp handling for consistency.Error Handling Enhancements:
startErrvariables with struct-level fields (startErr) inapp.goandcache/service_endpoint_hash_bucket.goto ensure consistent error tracking during startup processes. ([[1]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-6c4b6ed7dc8834cef100f50dae61c30ffe7775a3f3f6f5a557517cb740c44a2dR84-R86),[[2]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-f0036b631f291b5541037862f3c7e45eecad8d23360087fd1a01b24edf55a8e9R40-R42))startErrfield instead of local variables. ([[1]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-6c4b6ed7dc8834cef100f50dae61c30ffe7775a3f3f6f5a557517cb740c44a2dL207-R209),[[2]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-6c4b6ed7dc8834cef100f50dae61c30ffe7775a3f3f6f5a557517cb740c44a2dL247-R249),[[3]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-6c4b6ed7dc8834cef100f50dae61c30ffe7775a3f3f6f5a557517cb740c44a2dL270-R272),[[4]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-f0036b631f291b5541037862f3c7e45eecad8d23360087fd1a01b24edf55a8e9L92-R104),[[5]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-f0036b631f291b5541037862f3c7e45eecad8d23360087fd1a01b24edf55a8e9L130-R139))Test Refactoring:
health/check_test.goto support parallel execution and improve readability by usingt.Runsubtests. ([[1]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-3170350b5358ef098939d29c2ed8f811641424608163c69a44aea27b9e53f8c1L13-R19),[[2]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-3170350b5358ef098939d29c2ed8f811641424608163c69a44aea27b9e53f8c1L41-R49),[[3]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-3170350b5358ef098939d29c2ed8f811641424608163c69a44aea27b9e53f8c1L73-R81),[[4]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-3170350b5358ef098939d29c2ed8f811641424608163c69a44aea27b9e53f8c1L145-R153),[[5]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-3170350b5358ef098939d29c2ed8f811641424608163c69a44aea27b9e53f8c1L229-R239))[[1]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-3170350b5358ef098939d29c2ed8f811641424608163c69a44aea27b9e53f8c1L13-R19),[[2]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-3170350b5358ef098939d29c2ed8f811641424608163c69a44aea27b9e53f8c1L286-R380))Timestamp Handling:
timestampfunction withtime.Now().UTC()for consistent and reliable timestamp generation inhealth/check.go,health/checker.go, and related tests. ([[1]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-ada481bd2b6582ee48931f6d536520d9ea07a436b438c54ae7cd8bb04de4004dL68-R68),[[2]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-a7307efb619b1e13332a984622b29e0f17352bccb59dc3df84ce33e12d56c3b1L94-R94),[[3]](https://github.com/Jacobbrewer1/web/pull/75/files#diff-a7307efb619b1e13332a984622b29e0f17352bccb59dc3df84ce33e12d56c3b1L124-R124))Test Utility Additions:
ptrinhealth/checker_test.goto simplify pointer creation in tests. ([health/checker_test.goR15-R19](https://github.com/Jacobbrewer1/web/pull/75/files#diff-b34a9fde0fea4438a9aa024a22b7ca2e7b8ff4a2e2c1f59034e9cb043d617755R15-R19))Workflow Update:
.github/workflows/ci-code-approval.yml) to improve test robustness by detecting order-dependent issues. ([.github/workflows/ci-code-approval.ymlL67-R67](https://github.com/Jacobbrewer1/web/pull/75/files#diff-4c8bb8f5cf698728ae107fc054fb2460fb0be2c349f4c5353577be9c6add0c50L67-R67))