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

Ref count tests and others #50597

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

benlesh
Copy link
Contributor

@benlesh benlesh commented Feb 9, 2025

This is just a start... @domfarolino I just wanted to get your attention here. There might be more tests we can come up with, but here's a start.

Included are some ref counting tests, and also tests that ensure teardown always happens before complete or error is notified. (this sort of relates to finally as well)

Copy link
Member

@domfarolino domfarolino left a comment

Choose a reason for hiding this comment

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

This PR looks pretty malformed. It adds a bunch of files that I think already exist, so most of it is just conflicts :( Please clean it up and separate ref counted tests from everything else.

const target = new EventTarget();


}, "ref-counted event target should not be re-entrant")
Copy link
Member

Choose a reason for hiding this comment

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

.......

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird... sorry, I was in an airport... I'll clean this up. (This is why it's a "draft") haha

@benlesh
Copy link
Contributor Author

benlesh commented Feb 10, 2025

Just a heads up, this is a draft right now, I only added two or three tests to the observable-constructor tests, but on airport wifi I couldn't get the latest upstream to rebase on. :) I'll clean it up.

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.

3 participants