test: add TestFormatDateHelperElapsed

[julio-lopez]

Adds a stable test for the elapsed template function.

[julio-lopez]

@mickael-menu PTAL when you have a chance. Thank you!

[tjex]

hey @julio-lopez this test fix got addressed in #374. It all happened in a hurry and your contribution must've just been overlooked as your (this) initial PR is now a few months old. Our bad!

#374 only addressed an elapsed time of a year, and yours is hours/minutes/seconds. I'm not sure how robust that test needs to be, but perhaps if the change from #374 does need to be extended, it might be more safe to make one singular case for elapsed: year, day, hour, minute, second. So as to cover all time bases.

[julio-lopez]

@tjex The original PR had a "temporary" fix for the failing test. The fix in #374 is more robust.

This PR adds separate stable tests for other relative time units. It may be a good idea to have this test as well, but feel free to discard it if it does not add value.

[tjex]

@julio-lopez makes sense. I pad a pr to your branch with some extra tidying and focussed your tests on edge cases (ticking over from one elapsed time measure to the next)

Also refactored the test addition from #374 into the TestFormatDateHelperElapsed for better context.

Merging my branch, should also update this PR with the current state of zk-org/zk/main as there have been some other changes there since your commits.

[zalegrala]

Can you add one please that includes "last week", which I think should also work.

[julio-lopez]

@zalegrala Can you provide the test case please? FWICT, the result for < 7 days is the number of days, for 7 days is 1 week ago, and for >7 days it is 2 weeks.

I added the case for 2 weeks.

[zalegrala]

{
  elapsed: 7 * 24 * time.Hour,
  want:    "last week",
},

I believe this should pass without code changes.

[julio-lopez]

That is that specific test case, and it produces "1 week ago".
What am I missing?

[zalegrala]

Oh maybe I misunderstand. I thought the underlying library that was doing the transformation from string to date is what is being tested here. Feel free to proceed if the test case I mentioned above doesn't add additional coverage. The last week text is what is being used in some of my templates.

[tjex]

it's actually the other way around. The library is calculating the elapsed time and then returning a string.
We're just testing that each possible span of elapsed time, returns the correct string (i.e., elapsed time in natural language)

[julio-lopez]

@tjex Thanks for the feedback.

@julio-lopez makes sense. I pad a pr to your branch with some extra tidying and focussed your tests on edge cases (ticking over from one elapsed time measure to the next)

I added and modified the tests as you suggested.

Also refactored the test addition from #374 into the TestFormatDateHelperElapsed for better context.

Factored it out in a separate test.

In general, it'd make sense to keep the changes self contained, although, in this case these two changes are highly related.

Merging my branch, should also update this PR with the current state of zk-org/zk/main as there have been some other changes there since your commits.

Merging the main branch in the PR would have worked as well. Anyway, rebased the PR on top of main with the changes you suggested in separate commits to simplify reviewing.

PTAL. Thanks.

[julio-lopez]

@tjex Anything else that needs to be changed in order to merge this test? Thanks.

[tjex]

All looks great :) Thanks very much @julio-lopez