tools: Add protoxform tests

ci/do_ci.sh

@@ -291,6 +291,8 @@ elif [[ "$CI_TARGET" == "bazel.fuzzit" ]]; then
 elif [[ "$CI_TARGET" == "fix_format" ]]; then
   # proto_format.sh needs to build protobuf.
   setup_clang_toolchain
+  echo "protoxform_test..."
+  ./tools/protoxform_test.sh
   echo "fix_format..."
   ./tools/check_format.py fix
   ./tools/format_python_tools.sh fix

tools/BUILD

@@ -25,6 +25,15 @@ envoy_py_test_binary(
     ],
 )
 
+py_library(
+    name = "run_command",
+    srcs = [
+        "run_command.py",
+    ],
+    srcs_version = "PY3",
+    visibility = ["//visibility:public"],
+)
+
 envoy_cc_binary(
     name = "bootstrap2pb",
     srcs = ["bootstrap2pb.cc"],

tools/api_proto_plugin/utils.py

@@ -2,7 +2,7 @@
 import os
 
 
-def ProtoFileCanonicalFromLabel(label):
+def ProtoFileCanonicalFromLabel(label, repo_tag):
   """Compute path from API root to a proto file from a Bazel proto label.
 
   Args:
@@ -12,11 +12,11 @@ def ProtoFileCanonicalFromLabel(label):
     A string with the path, e.g. for @envoy_api//envoy/type/matcher:metadata.proto
     this would be envoy/type/matcher/matcher.proto.
   """
-  assert (label.startswith('@envoy_api//'))
-  return label[len('@envoy_api//'):].replace(':', '/')
+  assert (label.startswith(repo_tag))
+  return label[len(repo_tag):].replace(':', '/')
 
 
-def BazelBinPathForOutputArtifact(label, suffix, root=''):
+def BazelBinPathForOutputArtifact(label, suffix, repo_tag, root=''):
   """Find the location in bazel-bin/ for an api_proto_plugin output file.
 
   Args:
@@ -36,5 +36,5 @@ def BazelBinPathForOutputArtifact(label, suffix, root=''):
   # dependencies in the aspect above, they all look the same. So, just pick an
   # arbitrary match and we're done.
   glob_pattern = os.path.join(
-      root, 'bazel-bin/external/envoy_api/**/%s%s' % (ProtoFileCanonicalFromLabel(label), suffix))
+      root, 'bazel-bin/**/%s%s' % (ProtoFileCanonicalFromLabel(label, repo_tag), suffix))
   return glob.glob(glob_pattern, recursive=True)[0]

tools/check_format_test_helper.py

@@ -7,6 +7,7 @@
 
 from __future__ import print_function
 
+from run_command import runCommand
 import argparse
 import logging
 import os
@@ -21,30 +22,13 @@
 errors = 0
 
 
-# Echoes and runs an OS command, returning exit status and the captured
-# stdout+stderr as a string array.
-def runCommand(command):
-  stdout = []
-  status = 0
-  try:
-    out = subprocess.check_output(command, shell=True, stderr=subprocess.STDOUT).strip()
-    if out:
-      stdout = out.decode('utf-8').split("\n")
-  except subprocess.CalledProcessError as e:
-    status = e.returncode
-    for line in e.output.splitlines():
-      stdout.append(line)
-  logging.info("%s" % command)
-  return status, stdout
-
-
 # Runs the 'check_format' operation, on the specified file, printing
 # the comamnd run and the status code as well as the stdout, and returning
 # all of that to the caller.
 def runCheckFormat(operation, filename):
   command = check_format + " " + operation + " " + filename
-  status, stdout = runCommand(command)
-  return (command, status, stdout)
+  status, stdout, stderr = runCommand(command)
+  return (command, status, stdout + stderr)
 
 
 def getInputFile(filename, extra_input_files=None):
@@ -80,10 +64,10 @@ def fixFileExpectingSuccess(file, extra_input_files=None):
     print("FAILED:")
     emitStdoutAsError(stdout)
     return 1
-  status, stdout = runCommand('diff ' + outfile + ' ' + infile + '.gold')
+  status, stdout, stderr = runCommand('diff ' + outfile + ' ' + infile + '.gold')
   if status != 0:
     print("FAILED:")
-    emitStdoutAsError(stdout)
+    emitStdoutAsError(stdout + stderr)
     return 1
   return 0
 
@@ -92,23 +76,23 @@ def fixFileExpectingNoChange(file):
   command, infile, outfile, status, stdout = fixFileHelper(file)
   if status != 0:
     return 1
-  status, stdout = runCommand('diff ' + outfile + ' ' + infile)
+  status, stdout, stderr = runCommand('diff ' + outfile + ' ' + infile)
   if status != 0:
     logging.error(file + ': expected file to remain unchanged')
     return 1
   return 0
 
 
 def emitStdoutAsError(stdout):
-  logging.error("\n".join(line.decode('utf-8') for line in stdout))
+  logging.error("\n".join(stdout))
 
 
 def expectError(filename, status, stdout, expected_substring):
   if status == 0:
     logging.error("%s: Expected failure `%s`, but succeeded" % (filename, expected_substring))
     return 1
   for line in stdout:
-    if expected_substring in line.decode('utf-8'):
+    if expected_substring in line:
       return 0
   logging.error("%s: Could not find '%s' in:\n" % (filename, expected_substring))
   emitStdoutAsError(stdout)
@@ -283,4 +267,4 @@ def runChecks():
   if errors != 0:
     logging.error("%d FAILURES" % errors)
     exit(1)
-  logging.warning("PASS")
+  logging.warning("PASS")

tools/proto_format.sh

@@ -25,4 +25,4 @@ bazel build ${BAZEL_BUILD_OPTIONS} --//tools/api_proto_plugin:default_type_db_ta
   @envoy_api//docs:protos --aspects //tools/protoxform:protoxform.bzl%protoxform_aspect --output_groups=proto \
   --action_env=CPROFILE_ENABLED=1 --host_force_python=PY3
 
-./tools/proto_sync.py "$1" ${PROTO_TARGETS}
+./tools/proto_sync.py "$1" "@envoy_api" ${PROTO_TARGETS}

tools/proto_sync.py

@@ -59,7 +59,7 @@ def __init__(self, message):
         message)
 
 
-def LabelPaths(label, src_suffix):
+def LabelPaths(label, src_suffix, repo_tag):
   """Compute single proto file source/destination paths from a Bazel proto label.
 
   Args:
@@ -72,8 +72,8 @@ def LabelPaths(label, src_suffix):
       destination is a provisional path in the Envoy source tree for copying the
       contents of source when run in fix mode.
   """
-  src = utils.BazelBinPathForOutputArtifact(label, src_suffix)
-  dst = 'api/%s' % utils.ProtoFileCanonicalFromLabel(label)
+  src = utils.BazelBinPathForOutputArtifact(label, src_suffix, repo_tag=repo_tag)
+  dst = 'api/%s' % utils.ProtoFileCanonicalFromLabel(label, repo_tag)
   return src, dst
 
 
@@ -94,27 +94,27 @@ def SyncProtoFile(cmd, src, dst):
       raise RequiresReformatError('%s and %s do not match' % (src, dst))
 
 
-def SyncV2(cmd, src_labels):
+def SyncV2(cmd, src_labels, repo_tag):
   """Diff or in-place update v2 protos from protoxform.py Bazel cache artifacts."
 
   Args:
     cmd: 'check' or 'fix'.
     src_labels: Bazel label for source protos.
   """
   for s in src_labels:
-    src, dst = LabelPaths(s, '.v2.proto')
+    src, dst = LabelPaths(s, '.v2.proto', repo_tag)
     SyncProtoFile(cmd, src, dst)
 
 
-def SyncV3Alpha(cmd, src_labels):
+def SyncV3Alpha(cmd, src_labels, repo_tag):
   """Diff or in-place update v3alpha protos from protoxform.py Bazel cache artifacts."
 
   Args:
     cmd: 'check' or 'fix'.
     src_labels: Bazel label for source protos.
   """
   for s in src_labels:
-    src, dst = LabelPaths(s, '.v3alpha.proto')
+    src, dst = LabelPaths(s, '.v3alpha.proto', repo_tag)
     # Skip empty files, this indicates this file isn't modified in next version.
     if os.stat(src).st_size == 0:
       continue
@@ -260,10 +260,11 @@ def SyncBuildFiles(cmd):
 
 if __name__ == '__main__':
   cmd = sys.argv[1]
-  src_labels = sys.argv[2:]
+  repo_tag = sys.argv[2]
+  src_labels = sys.argv[3:]
   try:
-    SyncV2(cmd, src_labels)
-    SyncV3Alpha(cmd, src_labels)
+    SyncV2(cmd, src_labels, repo_tag)
+    SyncV3Alpha(cmd, src_labels, repo_tag)
     SyncBuildFiles(cmd)
   except ProtoSyncError as e:
     sys.stderr.write('%s\n' % e)

tools/protoxform/protoxform.py

@@ -535,4 +535,4 @@ def Main():
 
 
 if __name__ == '__main__':
-  Main()
+  Main()

tools/protoxform_test.sh

@@ -0,0 +1,13 @@
+#!/bin/bash
+
+rm -rf bazel-bin/tools
+
+declare -r PROTO_TARGETS=$(bazel query "labels(srcs, labels(deps, //tools/testdata/protoxform:protos))")
+
+BAZEL_BUILD_OPTIONS+=" --remote_download_outputs=all"
+
+bazel build ${BAZEL_BUILD_OPTIONS} --//tools/api_proto_plugin:default_type_db_target=//tools/testdata/protoxform:protos \
+  //tools/testdata/protoxform:protos --aspects //tools/protoxform:protoxform.bzl%protoxform_aspect --output_groups=proto \
+  --action_env=CPROFILE_ENABLED=1 --host_force_python=PY3
+
+./tools/protoxform_test_helper.py ${PROTO_TARGETS}

tools/protoxform_test_helper.py

@@ -0,0 +1,117 @@
+#!/usr/bin/env python3
+
+from run_command import runCommand
+
+import logging
+import os
+import re
+import sys
+
+
+def PathAndFilename(label):
+  """Retrieve actual path and filename from bazel label
+
+  Args:
+    label: bazel label to specify target proto.
+
+  Returns:
+    actual path and filename
+  """
+  if label.startswith('/'):
+    label = label.replace('//', '/', 1)
+  elif label.startswith('@'):
+    label = re.sub(r'@.*/', '/', label)
+  else:
+    return label
+  label = label.replace(":", "/")
+  splitted_label = label.split('/')
+  return ['/'.join(splitted_label[:len(splitted_label) - 1]), splitted_label[-1]]
+
+
+def GoldenProtoFile(path, filename, version):
+  """Retrieve golden proto file path. In general, those are placed in tools/testdata/protoxform.
+
+  Args:
+    path: target proto path
+    filename: target proto filename
+    version: api version to specify target golden proto filename
+
+  Returns:
+    actual golden proto absolute path 
+  """
+  base = "./"
+  base += path + "/" + filename + "." + version + ".gold"
+  return os.path.abspath(base)
+
+
+def ResultProtoFile(path, filename, version):
+  """Retrieve result proto file path. In general, those are placed in bazel artifacts.
+
+  Args:
+    path: target proto path
+    filename: target proto filename
+    version: api version to specify target result proto filename
+
+  Returns:
+    actual result proto absolute path
+  """
+  base = "./bazel-bin/"
+  delimited_path = path.split('/')
+  base += os.path.join(*delimited_path, "protos", *delimited_path)
+  base += "/{0}.{1}.proto".format(filename, version)
+  return os.path.abspath(base)
+
+
+def Diff(result_file, golden_file):
+  """Execute diff command with unified form
+
+  Args:
+    result_file: result proto file
+    golden_file: golden proto file
+
+  Returns:
+    output and status code
+  """
+  command = 'diff -u '
+  command += result_file + ' '
+  command += golden_file
+  status, stdout, stderr = runCommand(command)
+  return [status, stdout, stderr]
+
+
+def Run(path, filename, version):
+  """Run main execution for protoxform test
+
+  Args:
+    path: target proto path
+    filename: target proto filename
+    version: api version to specify target result proto filename
+
+  Returns:
+    result message extracted from diff command
+  """
+  message = ""
+  golden_path = GoldenProtoFile(path, filename, version)
+  test_path = ResultProtoFile(path, filename, version)
+
+  status, stdout, stderr = Diff(test_path, golden_path)
+
+  if status != 0:
+    message = '\n'.join([str(line) for line in stdout + stderr])
+
+  return message
+
+
+if __name__ == "__main__":
+  messages = ""
+  logging.basicConfig(format='%(message)s')
+  path, filename = PathAndFilename(sys.argv[1])
+  messages += Run(path, filename, 'v2')
+  messages += Run(path, filename, 'v3alpha')
+
+  if len(messages) == 0:
+    logging.warning("PASS")
+    sys.exit(0)
+  else:
+    logging.error("FAILED:\n{}".format(messages))
+    sys.exit(1)

tools/run_command.py

@@ -0,0 +1,10 @@
+import subprocess
+
+
+# Echoes and runs an OS command, returning exit status and the captured
+# stdout and stderr as a string array.
+def runCommand(command):
+  proc = subprocess.run([command], shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+
+  return proc.returncode, proc.stdout.decode('utf-8').split('\n'), proc.stderr.decode(
+      'utf-8').split('\n')