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

ci: Add SSE contract test support #25

Merged
merged 25 commits into from Jan 31, 2024
Merged

Conversation

eli-darkly
Copy link
Contributor

@eli-darkly eli-darkly commented Nov 13, 2021

See: https://github.com/launchdarkly/sse-contract-tests

An example of this running in CI is here. That workflow is currently failing due to problems that appear to be JRuby-specific (see below); in all of the non-JRuby jobs, everything passes and I believe the SSE implementation is now fully compliant with spec.

The two kinds of failures I'm seeing in JRuby (which I haven't yet filed separate stories for, because they're so odd that I need to rule out problems in the test logic first) appear to be problems in HTTP client behavior, not in SSE parsing:

  • It seems to be unable to detect a dropped connection if it's in the middle of parsing an event. (Test: "reconnection/discards partial messages on retry")
  • It sometimes stops receiving data from the stream if the data is being sent in 1-character chunks. In case there's something I don't understand about httprb, or a related known issue, I've filed Funny behavior of readpartial in JRuby when reading a slow stream httprb/http#703 to seek insight.

If we can fix those, then we will have much better test coverage in JRuby than before, since as described in #27, the mechanism we've been using to do end-to-end HTTP tests within the unit tests for this project was extremely unreliable in JRuby.

If you'd like to test this locally in any version of Ruby, clone this repo and also https://github.com/launchdarkly/sse-contract-tests. In ruby-eventsource, run make build-contract-tests and then make start-contract-test-service. Then, in sse-contract-tests, run make and then ./sse-contract-tests -url http://localhost:8000 - that's it.

@eli-darkly eli-darkly changed the base branch from master to eb/sc-136139/jruby-9.2-tests December 29, 2021 21:38
Base automatically changed from eb/sc-136139/jruby-9.2-tests to master December 30, 2021 01:30
@eli-darkly eli-darkly changed the title WIP: implement contract tests implement contract tests Dec 30, 2021
contract-tests/service.rb Outdated Show resolved Hide resolved
Comment on lines 8 to 9
streams = {}
streamCounter = 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these two are needed anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, that's a copy-paste mistake.

Co-authored-by: Matthew M. Keeler <keelerm84@gmail.com>
@eli-darkly eli-darkly closed this Feb 10, 2022
@eli-darkly eli-darkly deleted the eb/sc-125613/contract-tests branch February 10, 2022 22:40
@eli-darkly eli-darkly restored the eb/sc-125613/contract-tests branch February 10, 2022 22:44
@eli-darkly eli-darkly reopened this Feb 10, 2022
@keelerm84 keelerm84 changed the title implement contract tests ci: Add SSE contract test support Dec 14, 2023
@keelerm84 keelerm84 requested review from a team and removed request for bwoskow-ld and louis-launchdarkly December 14, 2023 16:22
@keelerm84 keelerm84 marked this pull request as ready for review December 14, 2023 16:22
@keelerm84
Copy link
Member

I have updated this PR so it runs on GH actions instead of circleci.

The problems Eli mentioned still seem to be present, so I've just disabled the contract tests in jruby for now. Might as well get some value from them now instead of waiting until the jruby stuff gets resolved (if it ever does).

@keelerm84 keelerm84 merged commit 3dd93df into main Jan 31, 2024
10 checks passed
@keelerm84 keelerm84 deleted the eb/sc-125613/contract-tests branch January 31, 2024 16:53
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.

None yet

3 participants