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

w: Format time and get cmdline #41

Merged
merged 7 commits into from
Apr 1, 2024

Conversation

fortifiedhill
Copy link
Contributor

This change allows w to display time in a similar way to the original procps w. Additionally, w now fetches the cmdline for each entry.

Later, the fetch_cmdline and possibly format_time functions could be seperated out into a shared library for future applications, such as ps or top.

This change allows w to display time in a similar way to the original
procps w. Additionally, w now fetches the cmdline for each entry.
Copy link

codecov bot commented Mar 27, 2024

Codecov Report

Attention: Patch coverage is 65.62500% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 45.82%. Comparing base (bfd6c87) to head (912fe92).

❗ Current head 912fe92 differs from pull request most recent head 50e9da9. Consider uploading reports for the commit 50e9da9 to get more accurate results

Files Patch % Lines
src/uu/w/src/w.rs 68.42% 3 Missing and 3 partials ⚠️
tests/by-util/test_w.rs 61.53% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
+ Coverage   45.53%   45.82%   +0.28%     
==========================================
  Files          15       15              
  Lines        1803     1831      +28     
  Branches      240      247       +7     
==========================================
+ Hits          821      839      +18     
- Misses        853      859       +6     
- Partials      129      133       +4     
Flag Coverage Δ
macos_latest 44.72% <65.62%> (+0.44%) ⬆️
ubuntu_latest 44.80% <15.62%> (-0.61%) ⬇️
windows_latest 0.13% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sylvestre
Copy link
Contributor

nice, could you please add a test? thanks

@fortifiedhill
Copy link
Contributor Author

fortifiedhill commented Mar 27, 2024

nice, could you please add a test? thanks

No problem, I've added a unit test for each new function. I did this instead of testing the actual output, because it is bound to change in the future and the unit tests may be helpful if/when these functions are moved into a different location.

@sylvestre
Copy link
Contributor

Sure and thanks but could you please add integration test in tests/ too? Thanks

It makes more since to make fetch_cmdline and format_time public
functions until they are moved into a library or seperate file and
just use integration tests from the start.
@fortifiedhill
Copy link
Contributor Author

Sure and thanks but could you please add integration test in tests/ too? Thanks

I went ahead and removed the unit tests and added integration tests into tests/ instead. It might be a good idea to treat them as public functions from the start and then move them into a library or separate file, when it is time.

@sylvestre
Copy link
Contributor

Please keep the function tests in the same file.
Add tests on the binary like
test_no_header

Unit tests are where they should be and there is an added test
that checks the output of w.
@fortifiedhill
Copy link
Contributor Author

Please keep the function tests in the same file. Add tests on the binary like test_no_header

I've put the unit tests where they should be and I've added an integration test in tests/ that checks the current output of w.

Cargo.toml Outdated Show resolved Hide resolved
The time library has been switched to chrono.
Also, the integration test for cmdline checking has been dropped,
as there is really no expectation of what it should be.
@fortifiedhill fortifiedhill force-pushed the w_format_time_and_cmdline branch from 912fe92 to 50e9da9 Compare April 1, 2024 04:32
@sylvestre sylvestre merged commit 28263dd into uutils:main Apr 1, 2024
7 of 13 checks passed
@fortifiedhill fortifiedhill deleted the w_format_time_and_cmdline branch April 1, 2024 20:49
Krysztal112233 pushed a commit to Krysztal112233/procps that referenced this pull request Apr 12, 2024
* w: Format time and get cmdline

This change allows w to display time in a similar way to the original
procps w. Additionally, w now fetches the cmdline for each entry.
Krysztal112233 pushed a commit to Krysztal112233/procps that referenced this pull request Apr 12, 2024
* w: Format time and get cmdline

This change allows w to display time in a similar way to the original
procps w. Additionally, w now fetches the cmdline for each entry.
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.

2 participants