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

add enablements to fix tape-4.x #312

Merged
merged 1 commit into from
May 15, 2020
Merged

add enablements to fix tape-4.x #312

merged 1 commit into from
May 15, 2020

Conversation

warner
Copy link
Contributor

@warner warner commented May 15, 2020

This adds FunctionPrototype.apply, as well as .message for all the
various Error prototypes, to the enablements list.

This allows tape-4.x to be imported after lockdown, as well as fixing tape's
t.throws() method.

refs #293, but does not close it because tape-5.x is still broken

Note: this currently fails local tests, I think because we have assertions that these items are not on the list. I'd appreciate some pointers as to how to fix these.

@warner warner requested a review from erights May 15, 2020 20:26
Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM

@warner
Copy link
Contributor Author

warner commented May 15, 2020

Marking as ready-for-review in the hopes of making it trigger CI (I didn't think draft-ness would inhibit CI). But I expect it to fail tests.

@warner warner marked this pull request as ready for review May 15, 2020 21:33
@warner
Copy link
Contributor Author

warner commented May 15, 2020

@erights: should I also add constructor and name to the other Error prototypes, to match the ones we now have on TypeError? And/or add toString to all of them, to match what we do on Error?

Also, I updated test/enable-property-overrides.test.js to pass again, but I noticed it's only examining Error and TypeError. Should I add tests for the other kinds of errors?

@erights
Copy link
Contributor

erights commented May 15, 2020

Not in this pr. I’ll take this on afterwards

@kriskowal
Copy link
Member

An update for NEWS.md would be welcome.

@warner warner requested a review from kriskowal May 15, 2020 22:37
Copy link
Member

@kriskowal kriskowal left a comment

Choose a reason for hiding this comment

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

I see.

This adds `FunctionPrototype.apply`, as well as `.message` for all the
various Error prototypes, to the enablements list.

This allows tape-4.x to be imported after lockdown, as well as fixing tape's
t.throws() method.

refs #293, but does not close it because tape-5.x is still broken
@warner warner merged commit f0e1138 into master May 15, 2020
@warner warner deleted the 293-fix-tape4 branch May 15, 2020 23:24
warner added a commit to Agoric/agoric-sdk that referenced this pull request May 28, 2020
with a (manually) patched SES-0.7.7 (incorporating
endojs/endo#312), we can also install-ses before
importing tape
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.

3 participants