Skip to content

Commit

Permalink
OSFileBasedUnixSubprocessTest fixes
Browse files Browse the repository at this point in the history
- Reduce the sleep time in testReadingFromStdoutAfterCommandFinishesDoesNotBlocksVM to 1 second.   There's no value in holding up the tests for 15 seconds and only depending on the process being finished.
- #numberOfOpenFiles only cound FIFO (pipes) as the tests are about redirecting stdio and counting all files isn't reliable (causes periodic failures on the CI)
- #getStreamsInfoForRunningTest update comments for Pharo 8
  • Loading branch information
akgrant43 committed Apr 5, 2020
1 parent f06defe commit 1a5c026
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 18 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
helpers
getStreamsInfoForRunningTest
"We obtain the open tmp files before the open files because apparently getting the entries of /tmp using
"We obtain the open tmp files before the open files because in Pharo 7 and ealier getting the entries of /tmp using
'/tmp' asFileReference entries
opens a file descriptor and lets the FD open. Thus #numberOfOpenFiles, using lsof, detects /tmp as an open file and makes the test fail.
This happens only in travis under Ubuntu.
"
leaves the /tmp file descriptor open. Thus #numberOfOpenFiles, using lsof, detects /tmp as an open file and makes the test fail.
Fixed in Pharo 8."
| openTmpFiles |
openTmpFiles := self numberOfExistingTempStreamFiles.
^ Array with: self numberOfOpenFiles with: openTmpFiles
Original file line number Diff line number Diff line change
@@ -1,20 +1,16 @@
helpers
numberOfOpenFiles
"This is estimation number. Not to be used for real code. IT's basically
to check the number of opened files (regular files, directories, pipes) at the beginning of a test and at the end, to validate we are not leaving behind opened files.
This should work in most Unix-like and Linux
"
"This is estimation number. Not to be used for real code. It's basically to check the number of opened FIFO files at the beginning of a test and at the end, to validate we are not leaving behind opened files.
This should work in most Unix-like and Linux systems.
Only FIFO files are checked as we can't control other threads, and it is redirecting stdio that is happening in the tests."
| tmpFileName openFiles |
tmpFileName := (FileSystem workingDirectory / 'openFilesByPharoVM-' , UUID new printString) fullName.
tmpFileName asFileReference ensureDelete.
self systemAccessor system: ('lsof -p ', OSSVMProcess vmProcess pid printString, ' > "', tmpFileName, '" 2>&1').
openFiles := tmpFileName asFileReference readStreamDo: [ :str | | lines |
"We avoid the first header line"
lines := str contents lines allButFirst sorted.

"We then ignore all file properties except for the file name"
lines collect: [ :each | each substrings last ]
lines := str contents lines.
"Count only pipes"
lines select: [ :each | each includesSubstring: 'FIFO' ]
].
tmpFileName asFileReference ensureDelete.
^ openFiles

^ openFiles size
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ testReadingFromStdoutAfterCommandFinishesDoesNotBlocksVM
streamsInfo := self getStreamsInfoForRunningTest.
command := self newCommand
command: '/bin/sleep';
arguments: (Array with: '15');
arguments: (Array with: '1');
redirectStdout;
runAndWait.

Expand Down

0 comments on commit 1a5c026

Please sign in to comment.