Add test coverage for product-details rebuild #1595
Conversation
|
|
||
|
|
||
| def mock_setup_working_copy(branch, url, secrets): | ||
| subprocess.check_call(["git", "clone", "-n", url, str(shipit_api.common.config.PRODUCT_DETAILS_DIR)]) |
There was a problem hiding this comment.
This is not great, the test should be self-contained, but it was easiest for now. There's a few other network requests done as part of the rebuild that should be mocked as well...
There was a problem hiding this comment.
Not ideal, but anything is better than nothing at this point...
There was a problem hiding this comment.
Maybe there could be a static data directory under tests? Then you could store a snapshot of product details data in there and init a git repo at test run-time. I think at that point you wouldn't need to mock the function (can probably use the local repo path as the url)
The raw data is about 32M:
git clone https://github.com/mozilla-releng/product-details.git
Cloning into 'product-details'...
... done.
du -sh product-details
44M product-details
rm -rf product-details/.git
du -sh product-details
32M product-details
Maybe it can even be an empty repo...
The network calls to HG for the l10n data are more likely to fail/hit rate limits. Those would be good to mock.
There was a problem hiding this comment.
I think I'll land this before I run out of steam, and try and improve later. We can probably get away with a lot less than 32M (maybe even git init an empty repo) and still get meaningful testing, but I'd rather not block on that given you two already reviewed this.
jcristau
force-pushed
the
test-product-details
branch
3 times, most recently
from
c6e7a72 to
29a2a80
Compare
|
|
||
|
|
||
| def mock_setup_working_copy(branch, url, secrets): | ||
| subprocess.check_call(["git", "clone", "-n", url, str(shipit_api.common.config.PRODUCT_DETAILS_DIR)]) |
There was a problem hiding this comment.
Not ideal, but anything is better than nothing at this point...
| primary_builds = json.load(f) | ||
| assert set(primary_builds["ach"].keys()) == {"133.0", "134.0b8", "134.0b9", "135.0a1"} | ||
|
|
||
| assert not list(parent.glob("l10n/Devedition-*")) |
There was a problem hiding this comment.
...and this covers #1596, I assume?
| beta = Release( | ||
| product="firefox", | ||
| branch="releases/mozilla-beta", | ||
| version="134.0b8", |
There was a problem hiding this comment.
If I understand correctly, this + having a newer DevEdition version should cover the issue that caused the original backout?
There was a problem hiding this comment.
Yes. I verified the test is failing without the "don't skip l10n for devedition entirely" commit with the "Too few firefox primary builds for FIREFOX_DEVEDITION" error.
|
|
||
|
|
||
| def mock_setup_working_copy(branch, url, secrets): | ||
| subprocess.check_call(["git", "clone", "-n", url, str(shipit_api.common.config.PRODUCT_DETAILS_DIR)]) |
There was a problem hiding this comment.
Maybe there could be a static data directory under tests? Then you could store a snapshot of product details data in there and init a git repo at test run-time. I think at that point you wouldn't need to mock the function (can probably use the local repo path as the url)
The raw data is about 32M:
git clone https://github.com/mozilla-releng/product-details.git
Cloning into 'product-details'...
... done.
du -sh product-details
44M product-details
rm -rf product-details/.git
du -sh product-details
32M product-details
Maybe it can even be an empty repo...
The network calls to HG for the l10n data are more likely to fail/hit rate limits. Those would be good to mock.
| primary_builds = json.load(f) | ||
| assert set(primary_builds["ach"].keys()) == {"133.0", "134.0b8", "134.0b9", "135.0a1"} | ||
|
|
||
| assert not list(parent.glob("l10n/Devedition-*")) |
jcristau
force-pushed
the
test-product-details
branch
from
29a2a80 to
02e62e7
Compare
Prompted by recent breakage; this is based on #1594.