Skip to content

test(observability-metrics-aws-cloudwatch): fix time-rotted default-step test#279

Merged
akila-i merged 2 commits into
openchoreo:mainfrom
akila-i:fix/metrics-cloudwatch-default-step-test
Jul 10, 2026
Merged

test(observability-metrics-aws-cloudwatch): fix time-rotted default-step test#279
akila-i merged 2 commits into
openchoreo:mainfrom
akila-i:fix/metrics-cloudwatch-default-step-test

Conversation

@akila-i

@akila-i akila-i commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Purpose

TestGetResourceMetricsAppliesDefaultStep hardcoded a fixed start time (2026-05-04) that flows into normalizeStandardMetricPeriod, which derives the minimum period from time.Since(start) against the real wall clock. Once the wall clock passed the 63-day retention threshold, the minimum period was bumped from 300s to 3600s, breaking the "default step" assertion.

Use a time.Now()-relative start time so the query window stays recent, matching the sibling tests (NormalizesSubMinuteStep, etc.) and keeping the test focused on the behavior it asserts.

Approach

Summarize the solution and implementation details.

Related Issues

Include any related issues that are resolved by this PR.

Checklist

  • Tests added or updated (unit, integration, etc.)
  • Samples updated (if applicable)

Remarks

Add any additional context, known issues, or TODOs related to this PR.

Summary by CodeRabbit

  • Chores

    • Updated the Google Cloud Logging observability package version to 0.1.1.
  • Tests

    • Improved test reliability by using the current UTC time when verifying default CloudWatch metric query periods.

…tep test

TestGetResourceMetricsAppliesDefaultStep hardcoded a fixed start time
(2026-05-04) that flows into normalizeStandardMetricPeriod, which derives
the minimum period from time.Since(start) against the real wall clock.
Once the wall clock passed the 63-day retention threshold, the minimum
period was bumped from 300s to 3600s, breaking the "default step" assertion.

Use a time.Now()-relative start time so the query window stays recent,
matching the sibling tests (NormalizesSubMinuteStep, etc.) and keeping the
test focused on the behavior it asserts.

Signed-off-by: Akila-I <akila.99g@gmail.com>
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 765970c0-4ab2-418e-9736-f007ad8e3fca

📥 Commits

Reviewing files that changed from the base of the PR and between aee086e and a6ae361.

📒 Files selected for processing (2)
  • observability-logs-gcp-cloudlogging/VERSION
  • observability-metrics-aws-cloudwatch/internal/cloudwatchmetrics/queries_test.go

📝 Walkthrough

Walkthrough

The changes bump the Cloud Logging package version from 0.1.0 to 0.1.1 and update a CloudWatch test to use the current UTC time instead of a fixed timestamp.

Changes

Package release metadata

Layer / File(s) Summary
Cloud Logging package version
observability-logs-gcp-cloudlogging/VERSION
Updates the package version from 0.1.0 to 0.1.1.

CloudWatch test timestamp

Layer / File(s) Summary
Dynamic test timestamp
observability-metrics-aws-cloudwatch/internal/cloudwatchmetrics/queries_test.go
Uses time.Now().UTC() to construct the StartTime and EndTime for the default-step test.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The Purpose is clear, but Approach, Related Issues, and Remarks are left as template placeholders, so the PR description is incomplete. Fill in the Approach section with the implementation details, and replace the placeholder text in Related Issues and Remarks with actual PR context.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the change and uses a valid conventional-commit style with a clear, specific subject.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Signed-off-by: Akila-I <akila.99g@gmail.com>
@akila-i akila-i marked this pull request as ready for review July 10, 2026 05:27
@akila-i akila-i merged commit 397acdc into openchoreo:main Jul 10, 2026
3 checks passed
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.

3 participants