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.

Sign up for GitHub

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

Jump to bottom

cli/ci: obfs4_test.go:80: not the error we expected mocked error #1750

Closed
bassosimone opened this issue Aug 20, 2021 · 1 comment
Closed

cli/ci: obfs4_test.go:80: not the error we expected mocked error #1750

bassosimone opened this issue Aug 20, 2021 · 1 comment
Assignees
@bassosimone
Labels
bug Something isn't working correctly priority/low Nice to have
Milestone
Sprint 47 - Carib...

Comments

@bassosimone
Copy link
Contributor

Describe the bug
The CI fails intermittently with this error:

--- FAIL: TestOBFS4DialerFailsWithConnectionErrorAndContextExpiration (0.00s)
Error:     obfs4_test.go:80: not the error we expected mocked error
FAIL
coverage: 99.3% of statements

See https://github.com/ooni/probe-cli/pull/446/checks?check_run_id=3381985435

To Reproduce
Run many CI runs per day: eventually, it will happen.

Expected behavior
The test always succeeds.

Screenshots
N/A

System information (please complete the following information):
GitHub Actions

Additional context
N/A
@bassosimone bassosimone added bug Something isn't working correctly priority/low Nice to have effort/XS labels Aug 20, 2021
@bassosimone bassosimone added this to the Sprint 46 - Happy Oyster milestone Aug 20, 2021
@bassosimone bassosimone self-assigned this Aug 20, 2021
@bassosimone bassosimone mentioned this issue Aug 20, 2021
Merged
3 tasks
@bassosimone
Copy link
Contributor Author

Relapse: https://github.com/ooni/probe-cli/pull/452/checks?check_run_id=3517300381

bassosimone added a commit to ooni/probe-cli that referenced this issue Sep 5, 2021
@bassosimone
fix(ptx/obfs4_test.go): avoid context-vs-normal-code race
d0a41dd 
We want to test whether we get the context failure if the error
generated inside normal code happens _after_ the context cancellation.

The best way to do that is to write code that is not racy. To this
end, we just need to pause normal code until we know that the context
has returned to the caller. We also need to ensure we do not leak
a goroutine, hence we use a WaitGroup to check that.

Fixes ooni/probe#1750
@bassosimone bassosimone mentioned this issue Sep 5, 2021
Merged
3 tasks
ainghazal pushed a commit to ainghazal/probe-cli that referenced this issue Mar 8, 2022
@bassosimone
ci: master only runs coverage and only with go1.17 (+ fix flaky test) (…
428fc66 
…ooni#452)

* fix: disable debianrepo build on master branch

This just mitigates ooni/probe#1741 and does
not fully address it, but I'd rather avoid delving into this problem until
I open a release/v3.11.0 branch and have to really fix this issue.

* fix: only run coverage using go1.17

This is the version of Go with which we are going to bless v3.11.0
therefore it's the only version of Go that matters.

Reference issue: ooni/probe#1738.

* fix(ptx/obfs4_test.go): avoid context-vs-normal-code race

We want to test whether we get the context failure if the error
generated inside normal code happens _after_ the context cancellation.

The best way to do that is to write code that is not racy. To this
end, we just need to pause normal code until we know that the context
has returned to the caller. We also need to ensure we do not leak
a goroutine, hence we use a WaitGroup to check that.

Fixes ooni/probe#1750
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
bug Something isn't working correctly priority/low Nice to have
Projects
None yet
Milestone
Sprint 47 - Caribbean Reef Octopus
Development

No branches or pull requests
1 participant
@bassosimone