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

Standardise test_run_commands test #279

Merged
merged 2 commits into from
Sep 20, 2022

Conversation

aloisklink
Copy link
Contributor

test_run_command() expects that the output of uname -s is always "Linux\n". However, this isn't right for two reasons:

  • That string isn't guaranteed to be NUL-terminated, so one day it might instead be Linux\n<some_junk_data_here>.
  • On other platforms like FreeBSD, uname -s does not print Linux.

Because of this, I've changed the test command to print "Hello World!", and to handle the NUL-terminator and some other stuff.

Document the parameters of process_callback_fn.

It seems that the params buf/count do **NOT** include a NUL terminator.
This means that you need to manaully use `strncpy()` to convert the
data to a useable string.
test_run_command() expects that the output of `uname -s`
is always "Linux\n". However, this isn't right for two reasons:
- That string isn't guaranteed to be NUL-terminated, so one day
  it might instead be `Linux\n<some_junk_data_here>`.
- On other platforms like FreeBSD, `uname -s` does not print Linux.

Because of this, I've changed the test command to print "Hello World!",
and to handle the NUL-terminator and some other stuff.
@aloisklink aloisklink added the refactor Refactoring code label Sep 19, 2022
@aloisklink aloisklink added this to the CheriBSD Support milestone Sep 19, 2022
@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #279 (12a396b) into main (888c7cd) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #279      +/-   ##
==========================================
+ Coverage   49.00%   49.04%   +0.03%     
==========================================
  Files         116      116              
  Lines       18487    18495       +8     
==========================================
+ Hits         9060     9070      +10     
+ Misses       9427     9425       -2     
Impacted Files Coverage Δ
src/utils/os.h 100.00% <ø> (ø)
tests/utils/test_os.c 98.24% <100.00%> (+0.64%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@mereacre mereacre left a comment

Choose a reason for hiding this comment

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

Great

@aloisklink aloisklink merged commit 08b9c32 into main Sep 20, 2022
@aloisklink aloisklink deleted the test/standardize-run-command-tests branch September 20, 2022 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants