Skip to content

Commit

Permalink
Whipper's version RegEX: support all valid scheme cases
Browse files Browse the repository at this point in the history
Initial work by ArchangeGabriel: #421

Fixes #420.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
  • Loading branch information
JoeLametta committed Dec 3, 2019
1 parent 47c62a9 commit d5bf83e
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion whipper/test/test_result_logger.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,14 @@ def testLogger(self):
# do not test on version line, date line, or SHA-256 hash line
self.assertListEqual(actualLines[2:-1], expectedLines[2:-1])

# RegEX updated to support all the 4 cases of the versioning scheme:
# https://github.com/pypa/setuptools_scm/#default-versioning-scheme
self.assertRegex(
actualLines[0],
re.compile((
r'Log created by: whipper '
r'[\d]+\.[\d]+\.[\d]+\.dev[\w\.\+]+ \(internal logger\)'
r'[\d]+\.[\d]+\.[\d]+(\+d\d{8}|\.dev[\w\.\+]+)? '
r'\(internal logger\)'
))
)
self.assertRegex(
Expand Down

2 comments on commit d5bf83e

@Freso
Copy link
Member

@Freso Freso commented on d5bf83e Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test doesn’t actually test all four cases though, it just makes the current .log file not break. We should have 3 additional .log files in addition to test_result_logger.log which the version string being formatted differently in each and test each of those. (Or have a separate function to test the version string so we could unit test it with values directly, without having to parse a whole .log file…)

@JoeLametta
Copy link
Collaborator Author

@JoeLametta JoeLametta commented on d5bf83e Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this is just the first step. I've done it this way to quickly solve #420 (only issue left assigned to milestone 0.9.0).
EDIT: Just opened an issue about this: #427.

Please sign in to comment.