bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

tag-release.py script needs a refactory

RESOLVED INCOMPLETE

Status

Release Engineering
General
RESOLVED INCOMPLETE
3 years ago
2 months ago

People

(Reporter: massimo, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
As part of understanding the impact of the pull timeout changes, we need to add tests on the tag-release.py script. Unfortunately, this script has a '-' in its name and so it becomes hard to import and mock its functions.

Probably the best approach here is to move the functions provided by tag-release.py to a separate library.
fwiw http://stackoverflow.com/questions/8350853/how-to-import-python-module-when-module-name-has-a-dash-or-hyphen-in-it has some solutions that don't involve a complete refactor.

Albeit a refactor/rename would likely be ideal here anyway.
(Reporter)

Comment 2

3 years ago
Created attachment 8580269 [details] [diff] [review]
[tools] Bug 1145165 - created release.tag library, added tests.patch

Thanks Callek for the link!

changes in this patch:

* moved getBumpCommitMessage, getTagCommitMessage, bump, tag, tagRepo and tagOtherRepo functions from scripts/release/tag-release.py to lib/python/release/tag.py
* added unittests (lib/python/mozilla_buildtools/test/test_release_tag.py)
* coverage is 46% (in tag.py) - everything  is tested except for closures - do we need to refactor these as well to increase the coverage?
Attachment #8580269 - Flags: review?(nthomas)
Comment on attachment 8580269 [details] [diff] [review]
[tools] Bug 1145165 - created release.tag library, added tests.patch

Sorry for the delay reviewing this. The file move seems fine to me, except for some cleanup in tag.py (see below). I'm not sure where you're going with the tests though, they seem a bit incomplete in terms of verifying the functionality. Perhaps I'm missing the point though.

>diff --git a/lib/python/mozilla_buildtools/test/test_release_tag.py b/lib/python/mozilla_buildtools/test/test_release_tag.py
>+class TestReleaseTag(unittest.TestCase):
>+    def test_getBumpCommitMessage(self):
>+    def test_getTagCommitMessage(self):

These two seem fine.

>+    def test_bump(self, m_bumpFile):
...
>+        with mock.patch('__builtin__.open', mock.mock_open(read_data=content)):
>+            self.assertTrue(bump(repo, bumpFiles, versionKey) is None)

Doesn't seem to verify the content is modified correctly, just that bump() doesn't error ?

>+    def test_tag(self, m_run_cmd):

A lot of overlap with test_getTagCommitMessage ?

>+    def test_tagRepo(self, m_aap, m_mercurial, m_retry):
...
>+        tagRepo(config=config, repo=repo, revision=revision, reponame=reponame,
>+                tags=tags, bumpFiles=bumpFiles, relbranch=relbranch,
>+                pushAttempts=pushAttempts)

Relies on tagRepo to raise an exception if something goes wrong ? Similar for test_tagOtherRepo.

>diff --git a/scripts/release/tag-release.py b/lib/python/release/tag.py
>similarity index 55%
>copy from scripts/release/tag-release.py
>copy to lib/python/release/tag.py

The first few lines are:

1 #!/usr/bin/env python
2 
3  import logging
4  from os import path
5  import subprocess
6  import sys
7  
8  sys.path.append(path.join(path.dirname(__file__), "../../lib/python"))
9  logging.basicConfig(
10     stream=sys.stdout, level=logging.INFO, format="%(message)s")
11 log = logging.getLogger(__name__)

* the hashbang (1) can definitely go from a module
* sys.path.append (2) is the responsibility of whatever is using the module
* there's no logging in the module, so 3 and 9-11 can go
Attachment #8580269 - Flags: review?(nthomas) → feedback+
(Reporter)

Updated

3 years ago
Assignee: mgervasini → nobody
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → INCOMPLETE
(Assignee)

Updated

2 months ago
Component: General Automation → General
Product: Release Engineering → Release Engineering
You need to log in before you can comment on or make changes to this bug.