Skip to content

test: add dataapi tests to ci and fix them #1764

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 11 commits into from
Jul 30, 2025

Conversation

samlaf
Copy link
Collaborator

@samlaf samlaf commented Jul 14, 2025

Why are these changes needed?

dataapi tests were not running in ci. Created a single integration-tests target which runs all of them. This way its impossible to forget one.

Also refactored some test targets in makefile to make distinction between each clearer. Having too many targets is just confusing, and leads to errors like this.

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

samlaf added 2 commits July 15, 2025 03:06
dataapi tests were not running in ci. Created a single integration-tests target which runs all of them. This way its impossible to forget one.
This way dataapi tests will now be running.
@samlaf samlaf requested review from dmanc and pschork July 14, 2025 19:11
@samlaf
Copy link
Collaborator Author

samlaf commented Jul 14, 2025

@pschork the dataapi tests are, unsurprisingly, failing. Can you take a look?

@@ -107,4 +81,5 @@ jobs:
"commit_sha": "https://github.com/Layr-Labs/eigenda/commit/${{ github.sha }}"
}
env:
# TODO: which channel is this being sent to?
Copy link
Contributor

Choose a reason for hiding this comment

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

eigenda-pr

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks: 122446d

samlaf added 4 commits July 15, 2025 03:15
Previous makefile target was broken, as it wasnt used in ci.
This fixes thue integration test in ci which was failing because anvil was failing to start.
Otherwise anvil was failing to start silently
@samlaf samlaf requested a review from dmanc July 29, 2025 06:02
Copy link

github-actions bot commented Jul 29, 2025

The latest Buf updates on your PR. Results from workflow Buf Proto / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJul 30, 2025, 2:30 PM

@samlaf samlaf enabled auto-merge July 30, 2025 14:30
@samlaf samlaf added this pull request to the merge queue Jul 30, 2025
Merged via the queue into master with commit ad93db6 Jul 30, 2025
30 checks passed
@samlaf samlaf deleted the test--add-dataapi-tests-to-ci-and-fix-them branch July 30, 2025 18:08
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