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

Timex.Format.FormatError - invalid format: { :formatter, :unsupported_token, :ms } #454

Closed
simpers opened this issue Aug 8, 2018 · 1 comment

Comments

@simpers
Copy link
Contributor

simpers commented Aug 8, 2018

Steps to reproduce

Currently using Timex version 3.4.0 through github directly using { :timex, github: "bitwalker/timex", tag: "3.4.0" }.

I was writing a unit test for our time formatting when I stumbled upon this. In my actual case format_string is a constant in the app itself, and not defined directly in the unit test. But other than that it is the same code as I have. The directive %f works, but is not what we need.

  • API function called
  • Arguments provided
test "Test formatting a timestamp with %L", _state do
    format_string = "%FT%T.%LZ"
    time = "2018-01-01T00:00:00.000Z"
    |> Timex.parse!(format_string, :strftime)
    |> Timex.format!(format_string, :strftime)
    assert time == "2018-01-01T00:00:00.000Z"
end

Description of issue

  • What are the expected results?
    As per the documentation describing the Strftime formatter there should be a directive, %L, that produces: milliseconds (000..999)

  • If you are certain this is a bug in Timex, do you have references I
    can use to check the algorithm/logic against? For instance, a well-established
    date/time library in another language, a paper, test suite, etc. It is by no
    means a requirement, but is immensely helpful in fixing bugs, and provides me
    extra data to use when building out the test suite

I'm sure it is a bug as there is documentation on it (perhaps by mistake, but then the documentation has a bug, I suppose). It is also quite a common way of formatting a timestamp so I would expect it to exist, as it does in other libraries (not sure of which one, but on the Java side gson formats it this way currently in our environment, and I am trying to match it up to this).

@simpers
Copy link
Contributor Author

simpers commented Aug 8, 2018

Noticed that there was a pull request #433 regarding this issue already. Didn't find it initially. So I guess this is a duplicate.

simpers pushed a commit to simpers/timex that referenced this issue Aug 8, 2018
…that is the point of the %L, I would assume. If that assumption incorrect, feedback is welcome.
simpers pushed a commit to simpers/timex that referenced this issue Aug 8, 2018
simpers pushed a commit to simpers/timex that referenced this issue Aug 9, 2018
simpers pushed a commit to simpers/timex that referenced this issue Aug 9, 2018
…that is the point of the %L, I would assume. If that assumption incorrect, feedback is welcome.
simpers pushed a commit to simpers/timex that referenced this issue Aug 9, 2018
simpers pushed a commit to simpers/timex that referenced this issue Aug 9, 2018
ckhrysze pushed a commit that referenced this issue Sep 17, 2018
* #454: Fixed this and added some tests. Fixed to 3 zeroes as that is the point of the %L, I would assume. If that assumption incorrect, feedback is welcome.

* #454: Added some other tests in order to not decrease the coverall report.

* #454: Easier to read clauses and more descriptive test descriptions, as per the feedback.
@ckhrysze ckhrysze closed this as completed Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants