Skip to content

Conversation

@jessealama
Copy link
Collaborator

@jessealama jessealama marked this pull request as draft January 7, 2022 08:01
@ptomato
Copy link
Collaborator

ptomato commented Jan 7, 2022

I don't think the brand checks in the id() accessors are ill-conceived, I think they need to be there... they are there in the specification.

Looks like some of the legacy tests will need to be adjusted or deleted, and we should add the failing test262 tests to the expected failure list (at least, until we get another test262 PR merged to update the test262 tests upstream.)

@ptomato ptomato added the no-spec-text PR can be ignored by implementors label Jan 11, 2022
@jessealama jessealama force-pushed the ensure-requireinternal-slot branch from 02406c4 to 9f0fdb1 Compare February 1, 2022 10:53
@jessealama
Copy link
Collaborator Author

OK I restored the branding tests that I thought were ill-conceived.

@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #1995 (016f8fa) into main (74ad6f3) will increase coverage by 0.00%.
The diff coverage is 80.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1995   +/-   ##
=======================================
  Coverage   94.50%   94.50%           
=======================================
  Files          19       19           
  Lines       11006    11011    +5     
  Branches     1599     1614   +15     
=======================================
+ Hits        10401    10406    +5     
  Misses        585      585           
  Partials       20       20           
Flag Coverage Δ
test262 81.31% <80.00%> (+0.16%) ⬆️
tests 88.73% <0.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
polyfill/lib/calendar.mjs 94.55% <50.00%> (-0.05%) ⬇️
polyfill/lib/timezone.mjs 93.63% <100.00%> (+0.12%) ⬆️
polyfill/lib/ecmascript.mjs 95.34% <0.00%> (+0.02%) ⬆️

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 74ad6f3...016f8fa. Read the comment docs.

@jessealama jessealama force-pushed the ensure-requireinternal-slot branch from 8ce7a2d to 9f0fdb1 Compare February 2, 2022 14:10
@jessealama jessealama marked this pull request as ready for review February 2, 2022 14:14
@jessealama jessealama requested a review from Ms2ger February 2, 2022 14:15
@Ms2ger Ms2ger enabled auto-merge (rebase) February 2, 2022 14:31
@Ms2ger Ms2ger disabled auto-merge February 2, 2022 14:38
@Ms2ger
Copy link
Collaborator

Ms2ger commented Feb 2, 2022

I started writing tests at https://github.com/tc39/test262/compare/main...Igalia:branding?expand=1, but it seems like some existing ones need changes as well, like built-ins/Temporal/TimeZone/prototype/toJSON/tostring-call.js.

@ptomato
Copy link
Collaborator

ptomato commented Feb 2, 2022

Yeah, looks like we will need to add expected failures for those tests until we can get the fix merged into test262.

jessealama and others added 3 commits February 8, 2022 16:54
New branding checks for `TimeZone` invalidate the assumption
underlying this test.
@ptomato ptomato force-pushed the ensure-requireinternal-slot branch from f9aa872 to 016f8fa Compare February 9, 2022 00:55
@ptomato ptomato merged commit 94833c0 into tc39:main Feb 9, 2022
@jessealama jessealama deleted the ensure-requireinternal-slot branch February 11, 2022 08:19
12wrigja added a commit to 12wrigja/temporal-polyfill that referenced this pull request Mar 4, 2022
12wrigja added a commit to 12wrigja/temporal-polyfill that referenced this pull request Mar 4, 2022
toJSON.

Reject '-000000' as an extended year value.

Bump test262.

Port of tc39/proposal-temporal#1995.

Port of tc39/proposal-temporal#1992
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-spec-text PR can be ignored by implementors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants