Skip to content

Fix tests #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 30, 2025
Merged

Fix tests #17

merged 3 commits into from
May 30, 2025

Conversation

lwr20
Copy link
Member

@lwr20 lwr20 commented May 30, 2025

No description provided.

@lwr20 lwr20 requested a review from Copilot May 30, 2025 17:23
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Updates the Elasticsearch document tests to use the new stats.ResultSummary type and reflect added DNSPerf config fields.

  • Replace map[int]float64 with stats.ResultSummary for timing metrics
  • Extend expected JSON to include new RunStress, TestDNSPolicy, NumTargetPods, and TargetType fields
  • Change empty summaries from null to {} in blank test
Comments suppressed due to low confidence (3)

pkg/elasticsearch/elasticsearch_test.go:71

  • [nitpick] The JSON keys for stats (e.g., min, max, avg, P50, P75, P90, P99, datapoints) mix casing conventions. Consider adopting a consistent naming style (all lowercase or camelCase) for clarity.
"dnsperf":{"LookupTime":{"min":0.001,"max":0.013641,"avg":0.004745,"P50":0.00364,"P75":0.004745,"P90":0.006914,"P99":0.013641,"datapoints":17}

pkg/elasticsearch/elasticsearch_test.go:71

  • [nitpick] The test uses a large inline JSON string, which can be brittle and hard to maintain. Consider constructing the expected document programmatically or using a golden file to make updates easier.
expectedDoc := `{"config":...`

pkg/elasticsearch/elasticsearch_test.go:87

  • The output shape for empty summaries changed from null to {}. This is a breaking API change for consumers; verify it’s intentional and update any downstream mappings or documentation accordingly.
"dnsperf":{"LookupTime":{},"ConnectTime":{},"DuplicateSYN":0

@lwr20 lwr20 changed the title Fix ES tests Fix tests May 30, 2025
@lwr20 lwr20 marked this pull request as ready for review May 30, 2025 17:52
@lwr20 lwr20 merged commit d8075cc into main May 30, 2025
6 checks passed
@lwr20 lwr20 deleted the lwr-fix-tests branch May 30, 2025 17:54
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.

1 participant