Skip to content

coverage: Also use clusterd binary when running SLT#19193

Merged
def- merged 1 commit into
MaterializeInc:mainfrom
def-:pr-coverage-clusterd-slt
May 10, 2023
Merged

coverage: Also use clusterd binary when running SLT#19193
def- merged 1 commit into
MaterializeInc:mainfrom
def-:pr-coverage-clusterd-slt

Conversation

@def-

@def- def- commented May 9, 2023

Copy link
Copy Markdown
Contributor

As noticed in #18966

Motivation

  • This PR fixes a previously unreported bug.

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • This PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way) and therefore is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:

@def- def- requested a review from a team as a code owner May 9, 2023 22:46
@ggevay

ggevay commented May 10, 2023

Copy link
Copy Markdown
Contributor

I have to admit that I don't understand the changes in this PR, but I rebased #18966 on top of this PR, and started a coverage run to see if it fixes the problem: https://buildkite.com/materialize/coverage/builds/92

@def-

def- commented May 10, 2023

Copy link
Copy Markdown
Contributor Author

I did the same and the result looks good to me now: https://buildkite.com/materialize/coverage/builds/91

@def- def- merged commit f2c1be3 into MaterializeInc:main May 10, 2023
@def- def- deleted the pr-coverage-clusterd-slt branch May 10, 2023 07:45
@def- def- mentioned this pull request May 10, 2023
5 tasks
@def-

def- commented May 10, 2023

Copy link
Copy Markdown
Contributor Author

I have to admit that I don't understand the changes in this PR, but I rebased #18966 on top of this PR, and started a coverage run to see if it fixes the problem: https://buildkite.com/materialize/coverage/builds/92

As an explanation for what this does: In the SLT test only the sqllogictest container is running, so we were only copying out the sqllogictest binary and aggregating the coverage information for it. Now we also copy out the clusterd binary from sqllogictest container and aggregate the coverage information for it too.

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