-
Notifications
You must be signed in to change notification settings - Fork 805
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
Add e2e integration tests for Postgres and remove 2dc runs #3532
Add e2e integration tests for Postgres and remove 2dc runs #3532
Conversation
@yux0 since you are removing 2DC code, should I just remove all the 2dc stuff here? so that it will save you some effort to do it in another PR. |
@@ -8,10 +8,6 @@ go get github.com/dmetzgar/goveralls | |||
# download cover files from all the tests | |||
mkdir -p build/coverage | |||
buildkite-agent artifact download "build/coverage/unit_cover.out" . --step ":golang: unit test" --build "$BUILDKITE_BUILD_ID" | |||
buildkite-agent artifact download "build/coverage/integ_cassandra_cover.out" . --step ":golang: integration test with cassandra" --build "$BUILDKITE_BUILD_ID" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are those useless? I assume we don't generate code coverage in integration tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I believe so.
And if you look at the list, it doesn't take ndc into account. I don't have permission, would you have help me kick off a run to see how the coverage would change?
Thanks. It would be great |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM thanks @longquanzheng
34d108f
to
e8256f7
Compare
@yux0 @andrewjdawson2016 because I don't have permission :( can you help trigger a buildkite when reviewing it? |
What changed?
Adding postgres to run with integration test suits
Why?
Follow up to a conversation in #3488 (comment)
How did you test it?
It's adding tests
Potential risks
No
Local run succeeded: