Skip to content

SLLS-546 Don't use system clocks in tests#708

Merged
tomshafir-sonarsource merged 1 commit into
masterfrom
task/ts/SLLS-546-dont-use-system-clocks-in-tests
May 22, 2026
Merged

SLLS-546 Don't use system clocks in tests#708
tomshafir-sonarsource merged 1 commit into
masterfrom
task/ts/SLLS-546-dont-use-system-clocks-in-tests

Conversation

@tomshafir-sonarsource
Copy link
Copy Markdown
Contributor

No description provided.

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown

hashicorp-vault-sonar-prod Bot commented May 22, 2026

SLLS-546

@sonar-review-alpha
Copy link
Copy Markdown

sonar-review-alpha Bot commented May 22, 2026

Summary

This PR replaces system clock calls with fixed timestamps across test files to eliminate non-determinism and improve test reliability.

Changes:

  • Instant.now()Instant.EPOCH (Java epoch: 1970-01-01)
  • System.currentTimeMillis()Instant.parse("2026-01-01T00:00:00Z").toEpochMilli() in ConnectedModeMediumTests (using a shared constant)
  • Affected files: CommandManagerTests, SonarLintVSCodeClientTests, TaintIssueTest, ConnectedModeMediumTests, DelegatingCellIssueTest, VersionedOpenNotebookTests

These changes ensure test behavior is consistent regardless of when tests are executed, making them more stable and faster (no ordering or timing dependencies).

What reviewers should know

Where to focus:

  • ConnectedModeMediumTests: The main file with the most changes. Note the new CURRENT_TIME constant at line 109 using a fixed 2026-01-01 date.
  • Other test files: All use Instant.EPOCH for timestamp fields, which is appropriate for mock/test data where the actual timestamp value is irrelevant.

Key design decisions:

  • Instant.EPOCH is used where timestamps are incidental to the test logic (just filling required fields)
  • ConnectedModeMediumTests uses a future-dated constant (2026-01-01) instead of epoch—verify this choice aligns with any time-sensitive assertions in that test class

What to verify:

  • Confirm no tests have assertions that depend on relative timestamps (e.g., comparing timestamps to "now")
  • Check whether the 2026-01-01 date in ConnectedModeMediumTests was chosen for a specific reason or could also use EPOCH

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

Copy link
Copy Markdown

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ✅

Clean, well-scoped PR. All Instant.now() and System.currentTimeMillis() usages in test files have been replaced — no remaining non-deterministic clock calls found.

One thing worth knowing for future work: ConnectedModeMediumTests uses a named constant (2026-01-01) instead of Instant.EPOCH. This is intentional — the same timestamp is threaded through both the initial pull response's queryTimestamp field and the changedSince=<value> query parameter in the incremental sync mock URL. The value needs to be self-consistent within each test, and a semantically meaningful named constant (CURRENT_TIME) makes the intent clearer than Instant.EPOCH. The codebase already uses Clock injection in LanguageClientLogger as the pattern for production-side time control; this PR correctly limits itself to the test side only.

🗣️ Give feedback

@sonarqube-next
Copy link
Copy Markdown

@tomshafir-sonarsource tomshafir-sonarsource merged commit dc7dce8 into master May 22, 2026
11 checks passed
@tomshafir-sonarsource tomshafir-sonarsource deleted the task/ts/SLLS-546-dont-use-system-clocks-in-tests branch May 22, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants