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

Fix "test_before_across_chunks" test #524

Merged

Conversation

stanislavlevin
Copy link
Contributor

Pipe like "openssl rand -base64 2097152 | head -n 500" leads to "Broken pipe"
error on some systems. This is an unexpected behavior. So errors must be
suppressed.

Fixes: #523

Pipe like "openssl rand -base64 2097152 | head -n 500" leads to "Broken pipe"
error on some systems. This is an unexpected behavior. So errors must be
suppressed.

Fixes: pexpect#523
@stanislavlevin
Copy link
Contributor Author

Hello, how can i fix coverage?
Please advise.

@bmwiedemann
Copy link

IMHO the coveralls result is bogus, because it does not recognize that the change is in a test (and tests always cover themselves in addition to the code under test)

I independently discovered and submitted the same patch with this description:

Without this patch, the test failed with openssl-1.0.2p
because 502 lines were captured with the extra lines being at the end:

140510355240592:error:02012020:system library:fflush:Broken pipe:bss_file.c:434:fflush()
140510355240592:error:20074002:BIO routines:FILE_CTRL:system lib:bss_file.c:436:

neither openssl-1.0.2j nor 1.1.0h needed this patch.

I also learned this:
The changes around the fflushing were intentional:
openssl/openssl#3348

It was a fix for:
openssl/openssl#3232

@takluyver
Copy link
Member

Thanks, seems reasonable. I don't know why coveralls reckons the coverage decreased, but it's only by a tiny amount, so I'm just going to ignore it.

@takluyver takluyver merged commit 462cf15 into pexpect:master Sep 6, 2018
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.

3 participants