Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: Add protoxform tests #9241

Merged
merged 23 commits into from
Dec 19, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions tools/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
22 changes: 17 additions & 5 deletions tools/api_proto_plugin/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -36,5 +36,17 @@ 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]


def ExtractRepoName(label):
"""Extract repository name from label"""
repo = ""
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
for i, ch in enumerate(label):
if label[i] == label[i + 1]:
repo += "//"
break
else:
repo += ch
return repo
2 changes: 1 addition & 1 deletion tools/check_format_test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,4 +283,4 @@ def runChecks():
if errors != 0:
logging.error("%d FAILURES" % errors)
exit(1)
logging.warning("PASS")
logging.warning("PASS")
2 changes: 1 addition & 1 deletion tools/proto_format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ bazel build ${BAZEL_BUILD_OPTIONS} @envoy_api//docs:protos --aspects \
tools/protoxform/protoxform.bzl%protoxform_aspect --output_groups=proto --action_env=CPROFILE_ENABLED=1 \
--action_env=TYPE_DB_PATH --host_force_python=PY3

./tools/proto_sync.py "$1" ${PROTO_TARGETS}
./tools/proto_sync.py "$1" "@envoy_api" ${PROTO_TARGETS}
21 changes: 11 additions & 10 deletions tools/proto_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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


Expand All @@ -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
Expand Down Expand Up @@ -257,10 +257,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)
Expand Down
2 changes: 1 addition & 1 deletion tools/protoxform/protoxform.py
Original file line number Diff line number Diff line change
Expand Up @@ -532,4 +532,4 @@ def Main():


if __name__ == '__main__':
Main()
Main()
20 changes: 20 additions & 0 deletions tools/protoxform_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/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 --strategy=protoxform=sandboxed,local"

bazel build ${BAZEL_BUILD_OPTIONS} //tools/testdata/protoxform:protos --aspects \
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
tools/type_whisperer/type_whisperer.bzl%type_whisperer_aspect --output_groups=types_pb_text \
--host_force_python=PY3
declare -x -r TYPE_DB_PATH="${PWD}"/source/common/config/api_type_db.generated.pb_text
bazel run ${BAZEL_BUILD_OPTIONS} //tools/type_whisperer:typedb_gen -- \
${PWD} ${TYPE_DB_PATH} ${PROTO_TARGETS}

bazel build ${BAZEL_BUILD_OPTIONS} //tools/testdata/protoxform:protos --aspects \
tools/protoxform/protoxform.bzl%protoxform_aspect --output_groups=proto --action_env=CPROFILE_ENABLED=1 \
--action_env=TYPE_DB_PATH --host_force_python=PY3

./tools/protoxform_test_helper.py ${PROTO_TARGETS}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if @lizan has any Bazel Fu to make the tests work as Bazel aspects.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you can write a rule that depends on aspect (like what type_database rule does depend on type_whisperer aspect), then define a test target on that. but we're good to go with this for now.

78 changes: 78 additions & 0 deletions tools/protoxform_test_helper.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
#!/usr/bin/env python3

from run_command import runCommand

import logging
import os
import re
import sys


def PathAndFilename(label):
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
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, file, version):
base = "./"
base += path + "/" + file + "." + version + ".gold"
return os.path.abspath(base)


def ResultProtoFile(path, file, version):
base = "./bazel-bin"
base += path + "/protos" + path + "/" + filename + "." + version + ".proto"
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
return os.path.abspath(base)


def Diff(result_file, golden_file):
command = 'diff -u '
command += result_file + ' '
command += golden_file + ' '
status, stdout = runCommand(command)
return [status, stdout]


def RunV3Alpha(path, filename):
message = ""
golden_path_v3 = GoldenProtoFile(path, filename, 'v3alpha')
test_path_v3 = ResultProtoFile(path, filename, 'v3alpha')

status, msg = Diff(test_path_v3, golden_path_v3)

if status != 0:
message = '\n'.join([str(line) for line in msg])

return message


def RunV2(path, filename):
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
message = ""
golden_path_v2 = GoldenProtoFile(path, filename, 'v2')
test_path_v2 = ResultProtoFile(path, filename, 'v2')
status, msg = Diff(test_path_v2, golden_path_v2)

if status != 0:
message = '\n'.join([str(line) for line in msg])

return message


if __name__ == "__main__":
messages = ""
logging.basicConfig(format='%(message)s')
path, filename = PathAndFilename(sys.argv[1])
messages += RunV2(path, filename)
messages += RunV3Alpha(path, filename)

if len(messages) == 0:
logging.warning("PASS")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are going to want to sys.exit() with different success/failure values here (was just looking again at this PR as I'm writing some simple golden source test as well right now). Thanks.

else:
logging.error("FAILED:\n{}".format(messages))
19 changes: 19 additions & 0 deletions tools/run_command.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import logging
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
import subprocess


# Echoes and runs an OS command, returning exit status and the captured
# stdout+stderr as a string array.
def runCommand(command):
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
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)
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
return status, stdout
9 changes: 9 additions & 0 deletions tools/testdata/protoxform/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
licenses(["notice"]) # Apache 2

proto_library(
name = "protos",
visibility = ["//visibility:public"],
deps = [
"//tools/testdata/protoxform/envoy/v2:protos",
],
)
9 changes: 9 additions & 0 deletions tools/testdata/protoxform/envoy/v2/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
licenses(["notice"]) # Apache 2

proto_library(
name = "protos",
srcs = [
"sample.proto",
],
visibility = ["//visibility:public"],
)
12 changes: 12 additions & 0 deletions tools/testdata/protoxform/envoy/v2/sample.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
syntax = "proto3";

package envoy.v2;

message Sample {
message Entry {
string key = 1;
string value = 2;
}

repeated Entry entries = 1;
}
17 changes: 17 additions & 0 deletions tools/testdata/protoxform/envoy/v2/sample.proto.v2.gold
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
syntax = "proto3";

package envoy.v2;

option java_outer_classname = "SampleProto";
option java_multiple_files = true;
Shikugawa marked this conversation as resolved.
Show resolved Hide resolved
option java_package = "io.envoyproxy.envoy.v2";

message Sample {
message Entry {
string key = 1;

string value = 2;
}

repeated Entry entries = 1;
}
23 changes: 23 additions & 0 deletions tools/testdata/protoxform/envoy/v2/sample.proto.v3alpha.gold
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
syntax = "proto3";

package envoy.v3alpha;

option java_outer_classname = "SampleProto";
option java_multiple_files = true;
option java_package = "io.envoyproxy.envoy.v3alpha";

import "udpa/api/annotations/versioning.proto";

message Sample {
option (udpa.api.annotations.versioning).previous_message_type = "envoy.v2.Sample";

message Entry {
option (udpa.api.annotations.versioning).previous_message_type = "envoy.v2.Sample.Entry";

string key = 1;

string value = 2;
}

repeated Entry entries = 1;
}
Loading