rebaseline_server: constrain setADir/setBDir to consistently refer to single directories, not lists
Before this CL, we supported multiple setADirs/setBDirs for some operations but
not others... now we consistently support a single pair of directories.
NOTRY=true
R=stephana@google.com
TBR=stephana
Author: epoger@google.com
Review URL: https://codereview.chromium.org/480293002
diff --git a/gm/rebaseline_server/compare_rendered_pictures.py b/gm/rebaseline_server/compare_rendered_pictures.py
index 91bce02..f1af2e0 100755
--- a/gm/rebaseline_server/compare_rendered_pictures.py
+++ b/gm/rebaseline_server/compare_rendered_pictures.py
@@ -85,7 +85,7 @@
"""
def __init__(self,
- setA_dirs, setB_dirs,
+ setA_dir, setB_dir,
setA_section, setB_section,
image_diff_db,
image_base_gs_url=DEFAULT_IMAGE_BASE_GS_URL, diff_base_url=None,
@@ -100,12 +100,12 @@
asynchronously warm up the ImageDiffDB cache but not fill in self._results.
Args:
- setA_dirs: list of root directories to copy all JSON summaries from,
- and to use as setA within the comparisons. These directories may be
- gs:// URLs, special "repo:" URLs, or local filepaths.
- setB_dirs: list of root directories to copy all JSON summaries from,
- and to use as setB within the comparisons. These directories may be
- gs:// URLs, special "repo:" URLs, or local filepaths.
+ setA_dir: root directory to copy all JSON summaries from, and to use as
+ setA within the comparisons. This directory may be specified as a
+ gs:// URL, special "repo:" URL, or local filepath.
+ setB_dir: root directory to copy all JSON summaries from, and to use as
+ setB within the comparisons. This directory may be specified as a
+ gs:// URL, special "repo:" URL, or local filepath.
setA_section: which section within setA to examine; must be one of
ALLOWED_SECTION_NAMES
setB_section: which section within setB to examine; must be one of
@@ -159,29 +159,23 @@
try:
setA_root = os.path.join(tempdir, 'setA')
setB_root = os.path.join(tempdir, 'setB')
- setA_repo_revision = None
- setB_repo_revision = None
- for source_dir in setA_dirs:
- self._copy_dir_contents(source_dir=source_dir, dest_dir=setA_root)
- # TODO(stephana): There is a potential race condition here... we copy
- # the contents out of the source_dir, and THEN we get the commithash
- # of source_dir. If source_dir points at a git checkout, and that
- # checkout is updated (by a different thread/process) during this
- # operation, then the contents and commithash will be out of sync.
- setA_repo_revision = self._get_repo_revision(
- source_dir=source_dir, assert_if_not=setA_repo_revision)
- for source_dir in setB_dirs:
- self._copy_dir_contents(source_dir=source_dir, dest_dir=setB_root)
- setB_repo_revision = self._get_repo_revision(
- source_dir=source_dir, assert_if_not=setB_repo_revision)
+ # TODO(stephana): There is a potential race condition here... we copy
+ # the contents out of the source_dir, and THEN we get the commithash
+ # of source_dir. If source_dir points at a git checkout, and that
+ # checkout is updated (by a different thread/process) during this
+ # operation, then the contents and commithash will be out of sync.
+ self._copy_dir_contents(source_dir=setA_dir, dest_dir=setA_root)
+ setA_repo_revision = self._get_repo_revision(source_dir=setA_dir)
+ self._copy_dir_contents(source_dir=setB_dir, dest_dir=setB_root)
+ setB_repo_revision = self._get_repo_revision(source_dir=setB_dir)
self._setA_descriptions = {
- results.KEY__SET_DESCRIPTIONS__DIR: setA_dirs,
+ results.KEY__SET_DESCRIPTIONS__DIR: setA_dir,
results.KEY__SET_DESCRIPTIONS__REPO_REVISION: setA_repo_revision,
results.KEY__SET_DESCRIPTIONS__SECTION: setA_section,
}
self._setB_descriptions = {
- results.KEY__SET_DESCRIPTIONS__DIR: setB_dirs,
+ results.KEY__SET_DESCRIPTIONS__DIR: setB_dir,
results.KEY__SET_DESCRIPTIONS__REPO_REVISION: setB_repo_revision,
results.KEY__SET_DESCRIPTIONS__SECTION: setB_section,
}
@@ -468,23 +462,17 @@
else:
shutil.copytree(source_dir, dest_dir)
- def _get_repo_revision(self, source_dir, assert_if_not=None):
+ def _get_repo_revision(self, source_dir):
"""Get the commit hash of source_dir, IF it refers to a git checkout.
Args:
source_dir: path to source dir (GS URL, local filepath, or a special
"repo:" URL type that points at a file within our Skia checkout;
only the "repo:" URL type will have a commit hash.
- assert_if_not: if not None, raise an Exception if source_dir has a
- commit hash and that hash is not equal to this
"""
if source_dir.lower().startswith(REPO_URL_PREFIX):
repo_dir = os.path.join(REPO_BASEPATH, source_dir[len(REPO_URL_PREFIX):])
- revision = subprocess.check_output(
+ return subprocess.check_output(
args=[git_utils.GIT, 'rev-parse', 'HEAD'], cwd=repo_dir).strip()
- if assert_if_not and revision != assert_if_not:
- raise Exception('found revision %s that did not match %s' % (
- revision, assert_if_not))
- return revision
else:
return None
diff --git a/gm/rebaseline_server/compare_rendered_pictures_test.py b/gm/rebaseline_server/compare_rendered_pictures_test.py
index d6006dc..c50c1a0 100755
--- a/gm/rebaseline_server/compare_rendered_pictures_test.py
+++ b/gm/rebaseline_server/compare_rendered_pictures_test.py
@@ -56,8 +56,8 @@
})
results_obj = compare_rendered_pictures.RenderedPicturesComparisons(
- setA_dirs=[os.path.join(self.temp_dir, setA_subdir)],
- setB_dirs=[os.path.join(self.temp_dir, setB_subdir)],
+ setA_dir=os.path.join(self.temp_dir, setA_subdir),
+ setB_dir=os.path.join(self.temp_dir, setB_subdir),
setA_section=gm_json.JSONKEY_ACTUALRESULTS,
setB_section=gm_json.JSONKEY_ACTUALRESULTS,
image_diff_db=imagediffdb.ImageDiffDB(self.temp_dir),
@@ -82,8 +82,8 @@
"""Use repo: URL to specify summary files."""
base_repo_url = 'repo:gm/rebaseline_server/testdata/inputs/skp-summaries'
results_obj = compare_rendered_pictures.RenderedPicturesComparisons(
- setA_dirs=[posixpath.join(base_repo_url, 'expectations')],
- setB_dirs=[posixpath.join(base_repo_url, 'actuals')],
+ setA_dir=posixpath.join(base_repo_url, 'expectations'),
+ setB_dir=posixpath.join(base_repo_url, 'actuals'),
setA_section=gm_json.JSONKEY_EXPECTEDRESULTS,
setB_section=gm_json.JSONKEY_ACTUALRESULTS,
image_diff_db=imagediffdb.ImageDiffDB(self.temp_dir),
diff --git a/gm/rebaseline_server/server.py b/gm/rebaseline_server/server.py
index 011428b..1bd5963 100755
--- a/gm/rebaseline_server/server.py
+++ b/gm/rebaseline_server/server.py
@@ -597,8 +597,8 @@
download_all_images = (
param_dict.get(LIVE_PARAM__DOWNLOAD_ONLY_DIFFERING, [''])[0].lower()
not in ['1', 'true'])
- setA_dirs = param_dict[LIVE_PARAM__SET_A_DIR]
- setB_dirs = param_dict[LIVE_PARAM__SET_B_DIR]
+ setA_dir = param_dict[LIVE_PARAM__SET_A_DIR][0]
+ setB_dir = param_dict[LIVE_PARAM__SET_B_DIR][0]
setA_section = self._validate_summary_section(
param_dict.get(LIVE_PARAM__SET_A_SECTION, [None])[0])
setB_section = self._validate_summary_section(
@@ -608,18 +608,18 @@
# the left (setA).
if ((setA_section == gm_json.JSONKEY_ACTUALRESULTS) and
(setB_section == gm_json.JSONKEY_EXPECTEDRESULTS)):
- setA_dirs, setB_dirs = setB_dirs, setA_dirs
+ setA_dir, setB_dir = setB_dir, setA_dir
setA_section, setB_section = setB_section, setA_section
# Are we comparing some actuals against expectations stored in the repo?
# If so, we can allow the user to submit new baselines.
is_editable = (
(setA_section == gm_json.JSONKEY_EXPECTEDRESULTS) and
- (setA_dirs[0].startswith(compare_rendered_pictures.REPO_URL_PREFIX)) and
+ (setA_dir.startswith(compare_rendered_pictures.REPO_URL_PREFIX)) and
(setB_section == gm_json.JSONKEY_ACTUALRESULTS))
results_obj = compare_rendered_pictures.RenderedPicturesComparisons(
- setA_dirs=setA_dirs, setB_dirs=setB_dirs,
+ setA_dir=setA_dir, setB_dir=setB_dir,
setA_section=setA_section, setB_section=setB_section,
image_diff_db=_SERVER.image_diff_db,
diff_base_url='/static/generated-images',
diff --git a/gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_repo_url/compare_rendered_pictures.json b/gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_repo_url/compare_rendered_pictures.json
index 1c81d5f..ada58a3 100644
--- a/gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_repo_url/compare_rendered_pictures.json
+++ b/gm/rebaseline_server/testdata/outputs/expected/compare_rendered_pictures_test.CompareRenderedPicturesTest.test_repo_url/compare_rendered_pictures.json
@@ -133,16 +133,12 @@
"isExported": true,
"schemaVersion": 5,
"setA": {
- "dir": [
- "repo:gm/rebaseline_server/testdata/inputs/skp-summaries/expectations"
- ],
+ "dir": "repo:gm/rebaseline_server/testdata/inputs/skp-summaries/expectations",
"repoRevision": "fake-repo-revision",
"section": "expected-results"
},
"setB": {
- "dir": [
- "repo:gm/rebaseline_server/testdata/inputs/skp-summaries/actuals"
- ],
+ "dir": "repo:gm/rebaseline_server/testdata/inputs/skp-summaries/actuals",
"repoRevision": "fake-repo-revision",
"section": "actual-results"
},
