timewarrior icon indicating copy to clipboard operation
timewarrior copied to clipboard

summary.t test and single digit weeks

Open slyon opened this issue 4 years ago • 4 comments

Hey @lauft I saw you applied a fix to the summary.t test to make sure it works properly in single digit weeks (1a6c30b93ae04406be6be4cb4f619b30f221fd7b). There is another interesting case, though, that happens even rarer:

Testing close to the year transition. In this case we have double digit and single digit weeks combined. I think we need to take care of this as well. Compare to this output, which happened during Ubuntu testing:

not ok 4 - summary.t: Summary should work with :all hint
# FAIL: AssertionError on file /tmp/autopkgtest.2VjSdd/autopkgtest_tmp/test/summary.t line 207 in test_with_all_hint: 'self.assertIn("""':
#       '
# Wk  Date       Day ID Tags    Start      End    Time   Total
# --- ---------- --- -- ---- -------- -------- ------- -------
# W53 2021-01-03 Sun @3 FOO  10:00:00 11:00:00 1:00:00 1:00:00
# W1 2021-01-04 Mon @2 BAR  10:00:00 11:00:00 1:00:00 1:00:00
# W1 2021-01-05 Tue @1 BAZ  10:00:00 11:00:00 1:00:00 1:00:00
# 
#                                                      3:00:00
# ' not found in '
# Wk  Date       Day ID Tags    Start      End    Time   Total
# --- ---------- --- -- ---- -------- -------- ------- -------
# W53 2021-01-03 Sun @3 FOO  10:00:00 11:00:00 1:00:00 1:00:00
# W1  2021-01-04 Mon @2 BAR  10:00:00 11:00:00 1:00:00 1:00:00
# W1  2021-01-05 Tue @1 BAZ  10:00:00 11:00:00 1:00:00 1:00:00
# 
#                                                      3:00:00
# 
# '

(https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_77e2ada1e7a84929a74ba3b87153c0ac/autopkgtest-hirsute/hirsute/amd64/t/timew/20210104_035924_cff59@/log.gz)

slyon avatar Jan 29 '21 08:01 slyon

Good catch! However, I wonder whether there is a better way to test this. The current path seems to become a bit tedious... 🤔

lauft avatar Jan 29 '21 17:01 lauft

Yes. Maybe it would be worth calculating once in the beginning if there are any double-digit weeks in the current test. If so, fill=3 else fill=2

Then use python's FormatSpec (https://docs.python.org/3/library/string.html#formatspec) to fill up to 2 or 3 characters with dashes or spaces accordingly..

slyon avatar Feb 01 '21 09:02 slyon

Is there any problem with using a regular expression to match in this case as well?

The below change allows faketime '2021-01-4T08:00:00' test/summary.t to work properly for me. Granted, it does not check that the first column is aligned, but is that what this test is testing for?

diff --git a/test/summary.t b/test/summary.t
index d9cd9411..aa51f951 100755
--- a/test/summary.t
+++ b/test/summary.t
@@ -208,19 +208,15 @@ W10 2017-03-09 Thu @4 Tag1        8:43:08  9:38:15 0:55:07
         week_now = now.isocalendar()[1]
         week_tomorrow = tomorrow.isocalendar()[1]

-        two_digit_week = (week_yesterday > 9 | week_now > 9 | week_tomorrow > 9)
-
-        self.assertIn("""
-Wk{6} Date       Day ID Tags    Start      End    Time   Total
---{7} ---------- --- -- ---- -------- -------- ------- -------
-W{3} {0:%Y-%m-%d} {0:%a} @3 FOO  10:00:00 11:00:00 1:00:00 1:00:00
-W{4} {1:%Y-%m-%d} {1:%a} @2 BAR  10:00:00 11:00:00 1:00:00 1:00:00
-W{5} {2:%Y-%m-%d} {2:%a} @1 BAZ  10:00:00 11:00:00 1:00:00 1:00:00
-
-{6}                                                    3:00:00
-""".format(yesterday, now, tomorrow,
-           week_yesterday, week_now, week_tomorrow,
-           " " if two_digit_week is True else "", "-" if two_digit_week is True else ""), out)
+        self.assertRegex(out, r"""
+Wk\s+Date\s+Day\s+ID\s+Tags\s+Start\s+End\s+Time\s+Total
+-+ -+ -+ -+ -+ -+ -+ -+ -+
+W{3}\s+{0:%Y-%m-%d} {0:%a} @3 FOO  10:00:00 11:00:00 1:00:00 1:00:00
+W{4}\s+{1:%Y-%m-%d} {1:%a} @2 BAR  10:00:00 11:00:00 1:00:00 1:00:00
+W{5}\s+{2:%Y-%m-%d} {2:%a} @1 BAZ  10:00:00 11:00:00 1:00:00 1:00:00
+
+\s+3:00:00
+""".format(yesterday, now, tomorrow, week_yesterday, week_now, week_tomorrow))

     def test_with_all_hint_and_first_interval_later_in_day(self):
         """Summary should handle :all hint with first interval that starts later in day than latest interval"""

sruffell avatar Apr 08 '21 12:04 sruffell

@sruffell I do not see a general problem to use regex, especially when we are testing for content not for alignment.

Maybe we should add some tests for alignment which can use predefined dates and use regex anywhere else... 🤔

lauft avatar Apr 09 '21 07:04 lauft