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

feat(trycmd): test only exit status with TRYCMD=status #376

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bryango
Copy link

@bryango bryango commented Jan 4, 2025

Closes #374 (my own issue).

This adds a new mode triggered by TRYCMD=status which only tests the exit status of commands, ignoring outputs. As proposed in #374, this would be useful when dealing with non-reproducible commands (where the output may change frequently). In that case, checking against the output is unrealistic, but it is still useful to validate that e.g. all the examples in README run successfully.

The implementation is mostly trivial and no public APIs are touched. I have taken the liberty of bumping the patch version number so that I can use this feature for myself more easily 😆

Also, bump the patch release number.
Comment on lines +366 to +369
let output = match mode {
Mode::OnlyStatus => output,
_ => self.validate_streams(output, step, substitutions),
};
Copy link
Author

Choose a reason for hiding this comment

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

The only non-trivial logic is here: we skip validate_streams when we have Mode::OnlyStatus.

@bryango bryango marked this pull request as ready for review January 4, 2025 12:31
@bryango
Copy link
Author

bryango commented Jan 10, 2025

Friendly ping @epage for comments & review 😆

@epage
Copy link
Contributor

epage commented Jan 10, 2025

Keep in mind that this was posted during the holidays from which I am still catching up. You can get an idea of where I am at through my updates.

Also, per the contributor guide, we ask that problems and solutions get discussed before going on to PRs. I won't be looking at the implementation (PR) until the Issue we have an agreed to solution over there.

@epage epage marked this pull request as draft January 10, 2025 16:36
@bryango
Copy link
Author

bryango commented Jan 10, 2025

I see. In my defense, I want the feature right now 😆 and the implementation is easy so I just did it. Actually, I am already running a project on it.

But I do understand that the implementation deserves more discussion. Please take this PR as a proposal and if you feel that the design is not ideal, feel free to close it (it is a small patch, no loss haha).

Also, you don't need to respond right away, please take your time. Happy holidays!

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.

feature: ignore outputs in trycmd, only check successful exit
2 participants