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

Editorial: Unreachable code or testing gap in RoundDuration with undefined relativeTo and unit = weeks or months #2648

Closed
justingrant opened this issue Aug 14, 2023 · 3 comments
Assignees
Milestone

Comments

@justingrant
Copy link
Collaborator

RoundDuration currently includes this text:

2. If unit is "year", "month", or "week", and relativeTo is undefined, then
  a. Throw a RangeError exception.

Looking at codecov, the "year" branch of this check is tested but the "month" and "week" branches are not.

For "month" and "week", AFAICT Temporal.Duration.prototype.total and Temporal.Duration.prototype.round are the only callers of RoundDuration which could possibly have relativeTo undefined and could use "month" or "week". And both of those callers will call UnbalanceDateDurationRelative before calling RoundDuration. UnbalanceDateDurationRelative includes the same validation and RangeError-throwing for a missing relativeTo, which explains the codecov result.

Should the test in RoundDuration be an assertion instead for "month" and "week"?

Or should UnbalanceDateDurationRelative validate "year" too and RoundDuration should change to an assertion in all cases, including "year"?

Or is there another caller of RoundDuration that doesn't call UnbalanceDateDurationRelative and which can trigger this RangeError?

@justingrant
Copy link
Collaborator Author

Meeting 2023-08-17: Let's leave this issue open until all the Duration-related outstanding changes are merged, then we can see if this is still unreachable. If it is, then change it to an assertion for "month" and "week" cases.

@ptomato ptomato self-assigned this Apr 17, 2024
ptomato added a commit that referenced this issue Apr 26, 2024
…tion

With the previous changes, the throwing step can no longer be reached.

Closes: #2648
@ptomato ptomato added this to the Stage 4 milestone Sep 20, 2024
@ptomato
Copy link
Collaborator

ptomato commented Sep 20, 2024

@ptomato ptomato closed this as completed Sep 20, 2024
@ptomato
Copy link
Collaborator

ptomato commented Sep 20, 2024

(That said, we should still look for other coverage gaps using codecov; but it seems like codecov stopped updating 4 months ago. Might be related to #2931.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants