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

chore: use a local instead of remote file for test #337

Merged
merged 1 commit into from Oct 29, 2022

Conversation

lukekarrys
Copy link
Contributor

I was able to replicate the same behavior from #332 by piping a readable
stream with a highWaterMark of 1 to extract. I confirmed this test
still failed without the fixes from #332 and now passes.

@webark
Copy link
Contributor

webark commented Oct 27, 2022

nice! that's good to know.

@lukekarrys
Copy link
Contributor Author

@webark I learned it while making this PR 😄 You're original PR was very helpful and I was fine to leave it as an https request. But I decided to investigate a little since I'd be getting notifications for a flaky test and stumbled upon this fix.

@wraithgar
Copy link
Contributor

it's probably good to still have a test that makes sure this works with a stream sent directly from http/https. I know nock can return streams.

@lukekarrys
Copy link
Contributor Author

lukekarrys commented Oct 29, 2022

Force pushed a new commit that adds another test mocked with nock.

I was able to replicate the same behavior from #332 by piping a readable
stream with a `highWaterMark` of 1 to extract. I confirmed this test
still failed without the fixes from #332 and now passes.
@lukekarrys lukekarrys merged commit 2e45b11 into main Oct 29, 2022
@lukekarrys lukekarrys deleted the lk/local-file-test branch October 29, 2022 18:36
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