#117 - fix data races

[mkenney]

Change type

What types of changes does your code introduce to go-chrome?

• [ ] Bugfix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [X] Other (please explain here)

Fixing data race issues in test mocks and API.

Checklist

• [x] I have read the CONTRIBUTING guidelines.
• [x] Lint and unit tests pass locally with my changes.
• [x] I have created tests which fail without the change (if possible) and prove my fix is effective or that my feature works.
• [x] I have added necessary documentation (if appropriate).

What does this implement/fix? Please explain these changes.

Fixes a longstanding issue with data races in the library

• Replaces the websocket mock with a service using net/http/httptest
• Fixes unaddressed data races in the library
• Adds -race to the CI test flags

These changes cause the Ci builds to take about 3x longer (~10 minutes for tip builds, ~3 minutes for other builds), I'll review the throttling behavior separately.

Does this close any currently open issues?

#117

Any relevant logs, error output, etc?

Refer to the output of go test -race ./...

Any other comments?

[canhlinh]

There are some confusions on the code. What is purpose of Stop, Disconnect functions ? Please clarify which purpose for them, to make the code cleaner

[canhlinh]

A lot of logging the good for debug, But it make the code a bit harder for reading.

[mkenney]

The Stop function stops the websocket read loop, while the Disconnect function disconnects the websocket. They don't need to be separated the way they but they are helpful when mocking socket connections in unit tests.

The change I'm making to use the httptest package should remove the need to mock the socket connections, I'll know more when I get further through the changes.

[mkenney]

So these changes are fixing the data races in the tests, but there's still an issue with stoping the process, it's getting hung on a channel with no listener. I'll investigate further when I have a moment.

[canhlinh]

I think the pain point is how you using muxer. It’s might create a lot of deadlocks in the lib

[mkenney]

I agree, I think I'm going to review the entire setup.