tc39 / test262

Official ECMAScript Conformance Test Suite
Other
2.39k stars 464 forks source link

undefined TimeZoneIANAName should not throw RangeError in *relativeto-sub-minute-offset tests. #3319

Open FrankYFTang opened 2 years ago

FrankYFTang commented 2 years ago

The following newly added tests by https://github.com/tc39/proposal-temporal/pull/1871/files

are not according to the current Temporal proposal:

'built-ins/Temporal/Duration/compare/relativeto-sub-minute-offset' 'built-ins/Temporal/Duration/prototype/add/relativeto-sub-minute-offset' 'built-ins/Temporal/Duration/prototype/round/relativeto-sub-minute-offset' 'built-ins/Temporal/Duration/prototype/subtract/relativeto-sub-minute-offset' 'built-ins/Temporal/Duration/prototype/total/relativeto-sub-minute-offset'

test/built-ins/Temporal/Duration/compare/relativeto-sub-minute-offset.js

relativeTo = "2000-01-01T00:00+00:44:30[+00:44:30.123456789]";
assert.throws(RangeError, () => Temporal.Duration.compare(duration1, duration2, { relativeTo }), "no other rounding is accepted for offset");

This is wrong because ParseTemporalRelativeToString will take "2000-01-01T00:00+00:44:30[+00:44:30.123456789]" as isoString and call ParseTemporalTimeZoneString and get Record { [[Z]]: false, [[OffsetString]]: "+00:44:30", [[Name]]: undefined }. as return

Notice in step 7 of https://tc39.es/proposal-temporal/#sec-temporal-parsetemporaltimezonestring

7. If name is not undefined, then

will not evaluated as true and therefore 7-a will not be run because name is defined in step 3

3. Let ... and name be the parts of isoString produced respectively by the ...
 TimeZoneIANAName productions, or undefined if not present.

in "2000-01-01T00:00+00:44:30[+00:44:30.123456789]" there are no TimeZoneIANAName ("+00:44:30.123456789" is a TimeZoneUTCOffsetName and NOT a TimeZoneIANAName), therefore name is undefined after step 3.

Same issue with the other tests I mentioned above

@ptomato @Ms2ger @jugglinmike @ljharb

ptomato commented 2 years ago

I think this is an issue with the Temporal proposal, not with the tests: https://github.com/tc39/proposal-temporal/issues/1924

FrankYFTang commented 2 years ago

oh, I see. I cannot tell which one is the intend. Do you have a PR for the spec change?

FrankYFTang commented 2 years ago

so... even if I change the implementation to define name as TimeZoneBracketedName instead of TimeZoneIANAName these test still fail because of the throw at step 7a of ParseTemporalTimeZoneString by IsValidTimeZoneName The IsValidTimeZoneName will return false for

Etc/GMT ASCIISign Hour

or

TimeZoneUTCOffsetName

so the test will fail