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

Correctness Testing 2.0 #85

Merged
merged 7 commits into from
Jan 13, 2025
Merged

Conversation

fredjoonpark
Copy link
Contributor

@fredjoonpark fredjoonpark commented Jan 13, 2025

Issue #, if available:

Description of changes:

Removed our deprecated correctness test suite in place of a new approach for correctness testing.

  • Added a new class Mockmetheus to send remote operations from Prometheus using snappy-encoded payloads. The previous testing approach included bringing up a Prometheus instance to seed initial testing data to run queries. This introduced conflicts as queries would fetch data from across both local and remote (Amazon Timestream) sources.
    Also, we had test cases that included functions and certain operators using client-side logic (which are not in scope of the Connector). It now focuses exclusively on the correctness of remote read and write operations.

  • Correctness tests are set up as a separate module since the underlying Prometheus dependency is using v2.5.0, and the parser was introduced as a package in a later released version.

  • Fixed bug where write requests without labels returned 500 instead of 422. While Prometheus always includes labels in remote writes, our Connector now handles the edge case properly.

  • Updated error codes to better reflect "not found" scenarios:

    • MissingDatabaseWithWriteError: Changed from 400 to 404
    • MissingTableWithWriteError: Changed from 400 to 404

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link
Contributor

@trevorbonas trevorbonas left a comment

Choose a reason for hiding this comment

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

Reviewed internally.

Copy link
Contributor

@forestmvey forestmvey left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

@forestmvey forestmvey merged commit 8917c42 into awslabs:main Jan 13, 2025
4 checks passed
@forestmvey forestmvey deleted the integ-correctness branch January 13, 2025 18:01
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