#646 Improve test coverage in koji/util
Merged by mikem. Opened by franzh.
franzh/koji issue642  into  master

Download 646.patch
  • add unit test for koji/util: (coverage 58%->99%)
  • minor fix: check if input string time is valid

minor fix: add epoch check in koji.util.checkForBuilds

Please remove this change.

While Koji records the epoch field of builds, it does not consider it for uniqueness. In Koji, there can be only one build of a given NVR.

In the places where this function is used, the build data to check is generated from a list of nvr strings that may or may not (often not) include an explicit epoch value. When the list includes plain NVRs, the parse_NVR function is is going to return a blank epoch value. With this change, Koji will never recognize a build with non-null epoch has been tagged unless the user explicitly included the epoch.

minor fix: check if input string time is valid

I'm curious what test value led to this change?

One of the new tests is failing for me:

======================================================================
FAIL: Test parseTime function
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mike/Devel/koji/koji/tests/test_lib/test_utils.py", line 511, in test_parseTime
    self.assertEqual(int(now.strftime('%s')), koji.util.parseTime(now.strftime("%Y-%m-%d %H:%M:%S")))
AssertionError: 1508274224 != 1508259824

difference is four hours, so appears to be a timezone issue.

the changes are mixing tabs and spaces in test_LazyValue(). In Koji, we use spaces, no tabs.

I recommend configuring your editor to show visible tabs. E.g. in vim I have:

set list lcs=tab:»· lcs+=trail:·

Looks like several tests fail in python3. Use 'make test3' to check.

minor fix: check if input string time is valid

I'm curious what test value led to this change?

Mike, original design will fail with invalid date like 2017-12-32 or time like 25:61:61

difference is four hours, so appears to be a timezone issue.

It is timezone issue.
I'll use calendar module to calculate UTC timestamp, that should be able to eliminate timezone influence.

3 new commits added

    • fix python3 compatible issues
  • check if input string time is valid
  • add unit test for koji/util:

1 new commit added

  • add unit tests for koji/util

parseTime is only used one place in Koji -- the getChangelogEntries hub call. It is used to convert the before and after parameters for the call, even though, AFAICT we never pass those parameters in any of the places we use the call in Koji itself. It's possible an external script might use these options.

With your change to parseTime, it will return None instead of raising an error. In getChangelogEntries, this will result in the call silently ignoring invalid input, which does not seem correct.

rebased onto 49e351a33c68935955004951cd601860ca00a8b9

@mikem

You're right, the calendar do raise exception if the input date is invalid. However I found it won't raise exception if time is invalid. The datetime.datetime has better date/time detections than calendar.

I modified the commits and let datetime raise exception if input date/time is invalid. The test cases are also updated as well.

rebased onto 1d1710636a080bd62a09d6a849cc4b35b1fa779d

:thumbsup:

Commit 7acaf4ce fixes this pull-request

Pull-Request has been merged by mikem@redhat.com

Metadata