-
Notifications
You must be signed in to change notification settings - Fork 5
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(tests): Various fixes for the testing framework #55
Conversation
|
2606952
to
dad6327
Compare
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
Please, check the guidelines for a commit message |
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.
Please use imperative mode in commit messages.
This is a series of 3 commits: tidy(tests): Remove unnecessary ticks This messes up some editor syntax highlighting fix(tests): Fix binary output being outputted The variable used in the conditional is different from the actual filename used. This conditional would always succeed since awk would return 0 if the conditional in awk would not match, which it didn't since stdout was empty (only stderr was populated with the error message, but it is disregarded). fix(tests): Coredumps not being generated At least 3 years ago, `sudo` had a change of behaviour regarding coredumps. This fixes that by setting the default for cores as the system.
Removing the backticks caused the output to not behave correctly. This fixes that by ensuring they remain on one line.
52eb0a5
to
f4ac140
Compare
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.
Please, reword the commit message (maybe you intend to squash both commits, but the current message refers other commits that are not there anymore). Also, notice some of these changes are not atomic (in a sense that if you need to revert a commit, it wouldn't produce collateral effects)
I've reworded it a bit so it looks like a full commit. On the last part, I mostly agree but these are 2 rather trivial changes not related to the main codebase (only relating to tests), so it's not worth the effort to split these apart, especially since the last point on the bullet list is dependent on the second. |
* Remove unnecessary ticks This messes up some editor syntax highlighting * Fix binary output being outputted The variable used in the conditional is different from the actual filename used. This conditional would always succeed since awk would return 0 if the conditional in awk would not match, which it didn't since stdout was empty (only stderr was populated with the error message, but it is disregarded). * Fix coredumps not being generated At least 3 years ago, `sudo` had a change of behavior regarding coredumps. This fixes that by setting the default for cores as the system.
This is a series of 3 commits:
tidy(tests): Remove unnecessary ticks
This messes up some editor syntax highlighting
fix(tests): Fix binary output being outputted
The variable used in the conditional is different from the actual
filename used. This conditional would always succeed since awk would
return 0 if the conditional in awk would not match, which it didn't
since stdout was empty (only stderr was populated with the error
message, but it is disregarded).
fix(tests): Coredumps not being generated
At least 3 years ago,
sudo
had a change of behaviour regardingcoredumps. This fixes that by setting the default for cores as the
system.