Fix(system_subprocess): save_completed_state passing stderr instead of stdin to callback#1159
Fix(system_subprocess): save_completed_state passing stderr instead of stdin to callback#1159Mahmood-Sinan wants to merge 3 commits intofortran-lang:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1159 +/- ##
==========================================
+ Coverage 68.00% 68.74% +0.73%
==========================================
Files 404 407 +3
Lines 12935 13641 +706
Branches 1392 1546 +154
==========================================
+ Hits 8797 9377 +580
- Misses 4138 4264 +126 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Nice catch, thank you for fixing that @Mahmood-Sinan. Would it be possible to add a test routine that exercises using stdin with a process completion callback ?
Thanks for reviewing. I have added the test for callback. I had to use a wrapper for payload because array of character strings were getting converted into a scalar variable in callback subroutine. Please take a look. |
jvdp1
left a comment
jvdp1
left a comment
There was a problem hiding this comment.
thank you @Mahmood-Sinan . Here are a few suggestions
test/system/test_subprocess.f90
Outdated
| module test_subprocess | ||
| use testdrive, only : new_unittest, unittest_type, error_type, check, skip_test | ||
| use stdlib_system, only: process_type, run, runasync, is_running, wait, update, elapsed, is_windows, kill | ||
| use stdlib_system, only: process_type, run, runasync, is_running, wait, update, elapsed, is_windows, kill, process_ID |
There was a problem hiding this comment.
| use stdlib_system, only: process_type, run, runasync, is_running, wait, update, elapsed, is_windows, kill, process_ID | |
| use stdlib_system, only: process_type, run, runasync, is_running, wait, update, elapsed, is_windows, kill, process_id |
|
|
||
| implicit none | ||
|
|
||
| type :: payload_wrapper |
There was a problem hiding this comment.
could be the DT string_type of stdlib used?
There was a problem hiding this comment.
When I tried, the compiler gave me an error saying that
193 | type is (string_type)
| 1
Error: The type-spec shall not specify a sequence derived type or a type with the BIND attribute in SELECT TYPE at (1) [F2003:C815]
I supposed we cannot do so, since string_type has sequence attribute.
test/system/test_subprocess.f90
Outdated
| end subroutine test_input_redirection | ||
|
|
||
| subroutine test_callback(error) | ||
|
|
test/system/test_subprocess.f90
Outdated
| end subroutine test_callback | ||
|
|
||
| subroutine callback_function(pid, exitcode, stdin, stdout, stderr, payload) | ||
| integer(process_ID), intent(in) :: pid |
There was a problem hiding this comment.
| integer(process_ID), intent(in) :: pid | |
| integer(process_id), intent(in) :: pid |
|
@jvdp1 Thanks for reviewing. I have made the changes. |
3 participants
Fixes incorrect argument mapping in
process_callbackinsidesave_completed_statesubroutine whereprocess%stderrwas passed twice, causingstdinto be omitted. In one placeprocess%stderrwas being passed in place ofprocess%stdin. This change correctly passesprocess%stdin.