Skip to content

Spec test requires different functions to compose with each other #726

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

Closed
catamorphism opened this issue Mar 13, 2024 · 2 comments · Fixed by #732
Closed

Spec test requires different functions to compose with each other #726

catamorphism opened this issue Mar 13, 2024 · 2 comments · Fixed by #732
Labels
functions Issue pertains to the default function set LDML45 LDML45 Release (Tech Preview) test-suite Issue pertains to tests

Comments

@catamorphism
Copy link
Collaborator

Right now the spec tests include a test for the :time and :datetime functions:

.input {$dt :time style=medium} {{{$dt :datetime dateStyle=long}}}

The expected output is: "January 2, 2006 at 3:04:06 PM".

To realize this expected output, the implementation would have to uphold the following properties in this specific instance:

  • :time returns an object that encodes the option style=medium as well as the formatter (:time) used to construct it
  • :datetime can inspect this object, check that it was constructed by :time, and treat the value of the style option as the value of its own timeStyle option

(:time can't just be treated as an alias to :datetime, since their options have different names.)

These requirements go beyond what #686 added to the spec, because they require different functions to compose with each other. @aphillips commented to that effect on #686: 'For now, we should basically say something like "functions can decide what operands to support" and "functions can decide what functions or function options to support".'

I don't think this test should be in the spec test suite at this time (perhaps there should be an appendix to the spec test suite to show what implementations can do if they choose to support more composability between functions), because it requires the implementation to support some pretty complex logic for which functions can compose with each other and how, without providing the machinery in the spec to guide the implementation of that logic. If the spec said something like, ":time has to return something that can be inspected to get the options and the name of the formatter used to construct it, and :datetime has to accept things created with :time or :date and use their options in combination with its own options", then it would be fine to require all implementations to produce the expected output here.

#645 could address this, but then there would also need to be specific language in the registry spec about the requirements for composability between :date/:time/:datetime.

@catamorphism catamorphism added functions Issue pertains to the default function set test-suite Issue pertains to tests labels Mar 13, 2024
@macchiati
Copy link
Member

macchiati commented Mar 13, 2024 via email

@aphillips
Copy link
Member

The expected output of this test is dead wrong, in my opinion. The meta-point is also correct: we shouldn't have a test like this. Let's just remove it.

@aphillips aphillips added the LDML45 LDML45 Release (Tech Preview) label Mar 17, 2024
aphillips added a commit that referenced this issue Mar 18, 2024
Fix #729: remove :time/:datetime compose test

Removes a questionable test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
functions Issue pertains to the default function set LDML45 LDML45 Release (Tech Preview) test-suite Issue pertains to tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants