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

Resolves issue where tests would not attempt to complete #236

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

KoryNunn
Copy link

This is a solution to the same problem that #235 addresses.

I reckon this is a better solution, but I'll leave them both open for review for now.

The new test fails in the current version of tape, and passes with the changes proposed.

@KoryNunn
Copy link
Author

KoryNunn commented Mar 1, 2016

I don't see this being addressed, so here is a workaround I'm using: https://gist.github.com/KoryNunn/a7ddbfb069bbbb2a41b4

@KoryNunn KoryNunn closed this Mar 1, 2016
@ljharb ljharb reopened this Mar 1, 2016
@ljharb
Copy link
Collaborator

ljharb commented Mar 1, 2016

This needs rebasing, but it's still worth evaluating I think?

@ljharb

This comment has been minimized.

@ljharb ljharb added the needs "allow edits" PR needs "allow edits" checked label Jan 9, 2019
@KoryNunn

This comment has been minimized.

@ljharb ljharb added semver-patch: bugfixes/etc and removed needs "allow edits" PR needs "allow edits" checked labels Jan 9, 2019
@ljharb
Copy link
Collaborator

ljharb commented Jan 9, 2019

@KoryNunn this has been rebased, but tests appear to be failing for me - tap.createConsumer doesn't seem to be available on the version of tap we use.

@KoryNunn
Copy link
Author

Looks like the changes break other tests, and after 3 years I'm not motivated enough to do anything about it.

Doesn't look like it has much requirement since no one else has been involved in this PR.

Someone else can have a go if they want.

@KoryNunn
Copy link
Author

I had a look at what the issue was and only the skip test fails. It looks like the outer tap ends before the tape test gets a chance to end it's output stream. I am not sure how to get around that.

@codecov-io
Copy link

codecov-io commented Jan 4, 2021

Codecov Report

Merging #236 (76ca27f) into master (b505c4c) will decrease coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage   74.15%   74.05%   -0.10%     
==========================================
  Files          19       19              
  Lines         766      771       +5     
  Branches      146      146              
==========================================
+ Hits          568      571       +3     
- Misses        198      200       +2     
Impacted Files Coverage Δ
lib/results.js 99.34% <100.00%> (-0.66%) ⬇️
lib/test.js 95.73% <0.00%> (-0.31%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b505c4c...76ca27f. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants