Skip to content

Commit

Permalink
Make CToolchain comparator ignore differences in CToolchain.Tool.tool…
Browse files Browse the repository at this point in the history
…_path and CToolcain.ToolPath.path when one is "" and the other "NOT_USED"

Starlark constructors for tool_path and tool do not allow empty strings for the path field. Therefore the migrator replaces the "" with "NOT_USED". We should ignore this difference when comparing the CToolchains.

#5380

PiperOrigin-RevId: 232827179
  • Loading branch information
scentini authored and copybara-github committed Feb 7, 2019
1 parent d4fef61 commit 903ad72
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 2 deletions.
11 changes: 9 additions & 2 deletions tools/migration/ctoolchain_comparator_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ def _check_with_feature_set_equivalence(before, after):


def _check_tool_equivalence(before, after):
"""Compares two "CToolchain.Tool"s."""
if before.tool_path == "NOT_USED":
before.tool_path = ""
if after.tool_path == "NOT_USED":
after.tool_path = ""
if before.tool_path != after.tool_path:
return False
if set(before.execution_requirement) != set(after.execution_requirement):
Expand Down Expand Up @@ -297,9 +302,11 @@ def _compare_tool_paths(tool_paths_before, tool_paths_after):
tool_to_path_before = {}
tool_to_path_after = {}
for tool_path in tool_paths_before:
tool_to_path_before[tool_path.name] = tool_path.path
tool_to_path_before[tool_path.name] = (
tool_path.path if tool_path.path != "NOT_USED" else "")
for tool_path in tool_paths_after:
tool_to_path_after[tool_path.name] = tool_path.path
tool_to_path_after[tool_path.name] = (
tool_path.path if tool_path.path != "NOT_USED" else "")

tool_names_before = set(tool_to_path_before.keys())
tool_names_after = set(tool_to_path_after.keys())
Expand Down
39 changes: 39 additions & 0 deletions tools/migration/ctoolchain_comparator_lib_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,45 @@ def test_action_config_implies_preserves_order(self):
self.assertIn("* Action config 'config' differs before and after",
mock_stdout.getvalue())

def test_unused_tool_path(self):
first = make_toolchain("""
tool_path {
name: "empty"
path: ""
}
""")
second = make_toolchain("""
tool_path {
name: "empty"
path: "NOT_USED"
}
""")
mock_stdout = StringIO()
with mock.patch("sys.stdout", mock_stdout):
compare_ctoolchains(first, second)
self.assertIn("No difference", mock_stdout.getvalue())

def test_unused_tool_path_in_tool(self):
first = make_toolchain("""
action_config {
config_name: 'config'
tool {
tool_path: ''
}
}
""")
second = make_toolchain("""
action_config {
config_name: 'config'
tool {
tool_path: 'NOT_USED'
}
}
""")
mock_stdout = StringIO()
with mock.patch("sys.stdout", mock_stdout):
compare_ctoolchains(first, second)
self.assertIn("No difference", mock_stdout.getvalue())

if __name__ == "__main__":
unittest.main()

0 comments on commit 903ad72

Please sign in to comment.