Skip to content
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

Enable response_parser_test.go #174

Merged
merged 8 commits into from
May 18, 2023
Merged

Enable response_parser_test.go #174

merged 8 commits into from
May 18, 2023

Conversation

fridgepoet
Copy link
Member

@fridgepoet fridgepoet commented May 17, 2023

What this PR does / why we need it:

  • enables the tests that were in response_parser_test.go_
  • updates the test assertions from using Points to now Frames and Fields
  • converts the assertions to use the testify library
  • renames a package from es to client

Discovered that some table to data frame conversion has not yet been done in this plugin. The tests that depend on that are now commented and an issue was created #175.

Which issue(s) this PR fixes:

Contributes to #117

Other information

Something I found interesting was that time in the "Points" used to be stored as float64 epoch time, so a "1000" time was the same as time.UnixMilli(1000).UTC() which is the same as time.Date(1970, time.January, 1, 0, 0, 1, 0, time.UTC). These are all typed now in the time-typed Fields.

@github-actions
Copy link

github-actions bot commented May 17, 2023

Backend code coverage report for PR #174

Plugin Main PR Difference
opensearch 43.4% 77.9% 34.5%

@github-actions
Copy link

Frontend code coverage report for PR #174
No changes

@github-actions
Copy link

github-actions bot commented May 17, 2023

Levitate is-compatible report:

🔍 Resolving @grafana/data@latest...
🔍 Resolving @grafana/ui@latest...
🔍 Resolving @grafana/runtime@latest...
🔍 Resolving @grafana/e2e-selectors@latest...

🔬 Checking compatibility between ./src/module.ts and @grafana/data@9.5.2...
✔ Found @grafana/data version 9.0.2 locally

🔬 Checking compatibility between ./src/module.ts and @grafana/ui@9.5.2...
✔ Found @grafana/ui version 9.0.2 locally

🔬 Checking compatibility between ./src/module.ts and @grafana/runtime@9.5.2...
✔ Found @grafana/runtime version 9.0.2 locally

🔬 Checking compatibility between ./src/module.ts and @grafana/e2e-selectors@9.5.2...
✔ Found @grafana/e2e-selectors version 9.0.2 locally

✔️ ./src/module.ts appears to be compatible with @grafana/data,@grafana/ui,@grafana/runtime,@grafana/e2e-selectors

@fridgepoet fridgepoet marked this pull request as ready for review May 17, 2023 10:15
@fridgepoet fridgepoet requested a review from a team as a code owner May 17, 2023 10:15
@fridgepoet fridgepoet requested review from iwysiu and idastambuk and removed request for a team May 17, 2023 10:15
Copy link
Contributor

@iwysiu iwysiu left a comment

Choose a reason for hiding this comment

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

Besides my note on what I think So(seriesOne.Points, ShouldHaveLength, 2) from the original is checking (I think it's checking the number of data points while you're checking the number of fields per data point) looks good!

pkg/opensearch/response_parser_test.go Outdated Show resolved Hide resolved
pkg/opensearch/response_parser_test.go Outdated Show resolved Hide resolved
Copy link
Member

@sarahzinger sarahzinger left a comment

Choose a reason for hiding this comment

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

🚀

@fridgepoet fridgepoet merged commit 89b627e into main May 18, 2023
5 checks passed
@fridgepoet fridgepoet deleted the test-response-parser branch May 18, 2023 15:39
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.

None yet

3 participants