Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Set pipefail for Cram tests #1636

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Set pipefail for Cram tests #1636

wants to merge 1 commit into from

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented Sep 23, 2024 •
edited
Loading

Description of proposed changes

This was added in places where it's been necessary. Setting it globally simplifies tests and allows future tests to use pipes without the need to set pipefail explicitly.

Related issue(s)

Checklist

@victorlin victorlin self-assigned this Sep 23, 2024
@victorlin victorlin changed the base branch from master to victorlin/cram-shared-setup September 23, 2024 22:53
@victorlin
Set pipefail in setup scripts
56e7657 
This was added in places where it's been necessary. Setting it in shared
setup scripts simplifies tests and allows future tests to use pipes
without the need to set pipefail explicitly.

A couple calls with non-zero exit codes have been exposed. These still
fall under expected behavior.
@victorlin victorlin marked this pull request as ready for review September 24, 2024 00:05
genehack
genehack approved these changes Sep 24, 2024
View reviewed changes
Copy link
Contributor

@genehack genehack left a comment

Choose a reason for hiding this comment

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

Is there any documentation around writing tests that needs to be updated to say "don't set pipefail yourself"?

Base automatically changed from victorlin/cram-shared-setup to master September 24, 2024 16:46
This was referenced Sep 24, 2024
Open
Merged
@victorlin
Copy link
Member Author

Is there any documentation around writing tests that needs to be updated to say "don't set pipefail yourself"?

How's #1639?

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@genehack genehack genehack approved these changes

Assignees

@victorlin victorlin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Set pipefail for cram tests
2 participants
@victorlin @genehack