Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#17085 closed enhancement (implemented)

Improve coverage on src/common/util_process.c

Reported by: rjunior Owned by:
Priority: Medium Milestone: Tor: 0.2.8.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: testing, 028-triaged
Cc: Actual Points:
Parent ID: Points: small
Reviewer: Sponsor: SponsorS

Description

The changes are in the branch "util_process_tests"

​​​​​​​https://github.com/twstrike/tor_for_patching/tree/util_process_tests

Child Tickets

Change History (9)

comment:1 Changed 4 years ago by rjunior

Status: newneeds_review

comment:2 Changed 4 years ago by nickm

Milestone: Tor: 0.2.8.x-final

comment:3 Changed 4 years ago by rl1987

Status: needs_reviewneeds_revision

Your test_util_process_clear_waitpid_callback() does not seem to checking for anything, unless you consider not crashing to be success. For some reason it calls clear_waitpid_callback() twice. Since you have some log capturing setup ready, maybe you should add some log statements to clear_waitpid_callback() and look for corresponding entries in the log to infer success/failure of clear_waitpid_callback() function.

comment:4 Changed 4 years ago by nickm

Keywords: 028-triaged added

comment:5 Changed 4 years ago by nickm

Sponsor: SponsorS

comment:6 Changed 4 years ago by nickm

Points: small

mark these testing tickets in needs_review as 'small work remaining'

comment:7 Changed 4 years ago by olabini

I have pushed fixes to the issues identified, and also fixed some space issues.

The reason why there are two calls to clear_waitpid_callback() is because the behavior is different if the callback has already been cleared. I now test the two cases by verifying that no logs were logged in the first case, and that a log happens after the second call.

comment:8 Changed 4 years ago by nickm

Resolution: implemented
Severity: Normal
Status: needs_revisionclosed

Merged!

There should eventually be more tests here, to ensure that we can actually monitor processes, but this is better than nothing.

comment:9 Changed 4 years ago by nickm

(Had to tweak the code to avoid valgrind errors.)

Note: See TracTickets for help on using tickets.