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 branding tests from Temporal #3138

Merged
merged 1 commit into from
Aug 11, 2021
Merged

Conversation

ptomato
Copy link
Contributor

@ptomato ptomato commented Aug 7, 2021

Closes: #3137

(Ms2ger originally authored all these tests so I'm making him the author of the commit)

Copy link
Member

@leobalter leobalter left a comment

Choose a reason for hiding this comment

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

Please remember to add the necessary metadata so the CI can pass. Failing to do it might break existing test262 runners.

description: >
  Throw a TypeError if this is invalid

The example above should be good enough.

I also believe this is not exactly branding tests, the tests are verifying if this is invalid. Maybe calling the files this-invalid-throws.js would be more consistent with other Test262 tests.

@leobalter
Copy link
Member

I've got corrected these are branding tests, I still have the nit pick over the test files but don't worry about changing the test file names.

@rwaldron
Copy link
Contributor

Please remember to add the necessary metadata so the CI can pass. Failing to do it might break existing test262 runners.

Yep! description is required (as the contributing documentation makes clear)

@ptomato ptomato force-pushed the 3137-branding-tests branch from 4eaa318 to ae27a83 Compare August 10, 2021 19:03
@ptomato
Copy link
Contributor Author

ptomato commented Aug 10, 2021

OK, thanks for the review! I've added descriptions to all the files.

@rwaldron rwaldron requested a review from leobalter August 11, 2021 16:00
@leobalter leobalter merged commit 45a913c into tc39:main Aug 11, 2021
@ptomato ptomato deleted the 3137-branding-tests branch August 11, 2021 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants