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

fix(ci): Make "test all" log output shorter #5521

Merged
merged 8 commits into from
Nov 4, 2022
Merged

fix(ci): Make "test all" log output shorter #5521

merged 8 commits into from
Nov 4, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Nov 1, 2022

Motivation

Sometimes GitHub truncates the log output of "test all", which makes it hard to find the actual failure.

This impacts some release candidate PRs, and it happens pretty often, so I just went ahead and fixed it.

Part of #5518, but we still need to fix the full sync logs.

Solution

  • Turn off --nocapture for "test all" in OS and Docker tests
  • Split Docker "test all" job into a basic and ignored-by-default step

Review

This is something @gustavovalverde and I discussed today.

Reviewer Checklist

  • Will the PR name make sense to users?
    • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
  • Are the PR labels correct?
  • Does the code do what the ticket and PR says?
  • How do you know it works? Does it have tests?

Follow Up Work

  • Fix the full sync logs

@teor2345 teor2345 added C-bug Category: This is a bug A-devops Area: Pipelines, CI/CD and Dockerfiles P-Medium ⚡ A-diagnostics Area: Diagnosing issues or monitoring performance labels Nov 1, 2022
@teor2345 teor2345 requested a review from a team as a code owner November 1, 2022 00:28
@teor2345 teor2345 self-assigned this Nov 1, 2022
@teor2345 teor2345 requested review from dconnolly and removed request for a team November 1, 2022 00:28
@github-actions github-actions bot added the C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG label Nov 1, 2022
@teor2345
Copy link
Contributor Author

teor2345 commented Nov 1, 2022

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Nov 1, 2022

update

✅ Branch has been successfully updated

@dconnolly
Copy link
Contributor

I lost the log snapshot but I restarted the CI jobs for 'disk full' messages on the OS jobs (linux and macos)

Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

Does not recognize --tag:

image

Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

Does not recognize --tag:

image

@teor2345
Copy link
Contributor Author

teor2345 commented Nov 1, 2022

Does not recognize --tag:

image

Thanks, the options should be fixed now.

@teor2345
Copy link
Contributor Author

teor2345 commented Nov 2, 2022

Opened bug #5532 and updated bug #5494 for test failures.

@teor2345
Copy link
Contributor Author

teor2345 commented Nov 2, 2022

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Nov 2, 2022

update

✅ Branch has been successfully updated

@teor2345
Copy link
Contributor Author

teor2345 commented Nov 3, 2022

It seems like @dconnolly is busy, @gustavovalverde would you mind reviewing this PR?

dconnolly
dconnolly previously approved these changes Nov 3, 2022
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

passing, looks good

mergify bot added a commit that referenced this pull request Nov 3, 2022
@dconnolly
Copy link
Contributor

Re-running the fail:

image

mergify bot added a commit that referenced this pull request Nov 4, 2022
@teor2345
Copy link
Contributor Author

teor2345 commented Nov 4, 2022

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Nov 4, 2022

update

☑️ Nothing to do

  • #commits-behind>0 [:pushpin: update requirement]
  • -closed [:pushpin: update requirement]

@teor2345 teor2345 requested a review from dconnolly November 4, 2022 02:21
dconnolly
dconnolly previously approved these changes Nov 4, 2022
@teor2345
Copy link
Contributor Author

teor2345 commented Nov 4, 2022

I merged in the changes from PR #5549, then only applied --nocapture to the on-by-default tests.

@teor2345
Copy link
Contributor Author

teor2345 commented Nov 4, 2022

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Nov 4, 2022

refresh

✅ Pull request refreshed

mergify bot added a commit that referenced this pull request Nov 4, 2022
@mergify mergify bot merged commit 2dc8c0a into main Nov 4, 2022
@mergify mergify bot deleted the better-ci-debug branch November 4, 2022 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles A-diagnostics Area: Diagnosing issues or monitoring performance C-bug Category: This is a bug C-trivial Category: A trivial change that is not worth mentioning in the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants