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

diff_test: add rule and tests #136

Merged
merged 6 commits into from
Apr 12, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions docs/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,10 @@ stardoc(
input = "//rules:write_file.bzl",
deps = ["//rules:write_file"],
)

stardoc(
name = "diff_test_docs",
out = "diff_test_doc_gen.md",
input = "//rules:diff_test.bzl",
deps = ["//rules:diff_test"],
)
5 changes: 5 additions & 0 deletions rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ bzl_library(
deps = ["//rules/private:write_file_private"],
)

bzl_library(
name = "diff_test",
srcs = ["diff_test.bzl"],
)

# Exported for build_test.bzl to make sure of, it is an implementation detail
# of the rule and should not be directly used by anything else.
exports_files(["empty_test.sh"])
Expand Down
145 changes: 145 additions & 0 deletions rules/diff_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
# Copyright 2019 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""A test rule that compares two binary files.

The rule uses a Bash command (diff) on Linux/macOS/non-Windows, and a cmd.exe
command (fc.exe) on Windows (no Bash is required).
"""

def _runfiles_path(f):
if f.root.path:
return f.path[len(f.root.path) + 1:] # generated file
else:
return f.path # source file

def _diff_test_impl(ctx):
if ctx.attr.is_windows:
test_bin = ctx.actions.declare_file(ctx.label.name + "-test.bat")
ctx.actions.write(
output = test_bin,
content = r"""@echo off
Copy link
Collaborator

Choose a reason for hiding this comment

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

I find it a little awkward to have this much content written as a string literal in Starlark. Could we move it to a bat file and use that as an action input?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried, but couldn't: I created templates under //rules/private and added a label attribute to _diff_test that select()s from them:

    diff_script_template = select({
        "@bazel_tools//src/conditions:host_windows": "//rules/private:diff_test_tmpl.bat",
        "//conditions:default": "//rules/private:diff_test_tmpl.sh",
    }),

Unfortunately the rule can't find the templates when the rule itself is in an external repo.

If you don't mind, I'd leave the ugly strings here; that approach works.

I edited a test to pull the rule from an external repo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand the issue you're running into ...

  1. It's not necessary to select on the diff script -- you can depend on the scripts for both windows and non-windows, and only actually use the script you need based on the value of is_windows.
  2. Depending on a script "tool" dependency is a pretty normal pattern for rules. If there's a serious problem with adopting this pattern from this repository, we should investigate.

That said, I know this review has been going slowly, so I'm fine with proceeding as-is and following up later. It should be an easy cleanup.

SETLOCAL ENABLEEXTENSIONS
SETLOCAL ENABLEDELAYEDEXPANSION
set MF=%RUNFILES_MANIFEST_FILE:/=\%
set PATH=%SYSTEMROOT%\system32
set F1={file1}
set F2={file2}
if "!F1:~0,9!" equ "external/" (set F1=!F1:~9!) else (set F1=!TEST_WORKSPACE!/!F1!)
if "!F2:~0,9!" equ "external/" (set F2=!F2:~9!) else (set F2=!TEST_WORKSPACE!/!F2!)
for /F "tokens=2* usebackq" %%i in (`findstr.exe /l /c:"!F1! " "%MF%"`) do (
set RF1=%%i
set RF1=!RF1:/=\!
)
if "!RF1!" equ "" (
echo>&2 ERROR: !F1! not found
exit /b 1
)
for /F "tokens=2* usebackq" %%i in (`findstr.exe /l /c:"!F2! " "%MF%"`) do (
set RF2=%%i
set RF2=!RF2:/=\!
)
if "!RF2!" equ "" (
echo>&2 ERROR: !F2! not found
exit /b 1
)
fc.exe 2>NUL 1>NUL /B "!RF1!" "!RF2!"
if %ERRORLEVEL% neq 0 (
if %ERRORLEVEL% equ 1 (
echo>&2 FAIL: files "{file1}" and "{file2}" differ
exit /b 1
) else (
fc.exe /B "!RF1!" "!RF2!"
exit /b %errorlevel%
)
)
""".format(
file1 = _runfiles_path(ctx.file.file1),
file2 = _runfiles_path(ctx.file.file2),
),
is_executable = True,
)
else:
test_bin = ctx.actions.declare_file(ctx.label.name + "-test.sh")
ctx.actions.write(
output = test_bin,
content = r"""#!/bin/bash
set -euo pipefail
F1="{file1}"
F2="{file2}"
[[ "$F1" =~ external/* ]] && F1="${{F1#external/}}" || F1="$TEST_WORKSPACE/$F1"
[[ "$F2" =~ external/* ]] && F2="${{F2#external/}}" || F2="$TEST_WORKSPACE/$F2"
if [[ -d "${{RUNFILES_DIR:-/dev/null}}" && "${{RUNFILES_MANIFEST_ONLY:-}}" != 1 ]]; then
RF1="$RUNFILES_DIR/$F1"
RF2="$RUNFILES_DIR/$F2"
elif [[ -f "${{RUNFILES_MANIFEST_FILE:-/dev/null}}" ]]; then
RF1="$(grep -F -m1 "$F1 " "$RUNFILES_MANIFEST_FILE" | sed 's/^[^ ]* //')"
RF2="$(grep -F -m1 "$F2 " "$RUNFILES_MANIFEST_FILE" | sed 's/^[^ ]* //')"
else
echo >&2 "ERROR: could not find \"{file1}\" and \"{file2}\""
exit 1
fi
if ! diff "$RF1" "$RF2"; then
echo >&2 "FAIL: files \"{file1}\" and \"{file2}\" differ"
exit 1
fi
""".format(
file1 = _runfiles_path(ctx.file.file1),
file2 = _runfiles_path(ctx.file.file2),
),
is_executable = True,
)
return DefaultInfo(
executable = test_bin,
files = depset(direct = [test_bin]),
runfiles = ctx.runfiles(files = [test_bin, ctx.file.file1, ctx.file.file2]),
)

_diff_test = rule(
attrs = {
"file1": attr.label(
allow_single_file = True,
mandatory = True,
),
"file2": attr.label(
allow_single_file = True,
mandatory = True,
),
"is_windows": attr.bool(mandatory = True),
},
test = True,
implementation = _diff_test_impl,
)

def diff_test(name, file1, file2, **kwargs):
"""A test that compares two files.

The test succeeds if the files' contents match.

Args:
name: The name of the test rule.
file1: Label of the file to compare to <code>file2</code>.
file2: Label of the file to compare to <code>file1</code>.
**kwargs: The <a href="https://docs.bazel.build/versions/master/be/common-definitions.html#common-attributes-tests">common attributes for tests</a>.
"""
_diff_test(
name = name,
file1 = file1,
file2 = file2,
is_windows = select({
"@bazel_tools//src/conditions:host_windows": True,
"//conditions:default": False,
}),
**kwargs
)
42 changes: 42 additions & 0 deletions tests/diff_test/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# This package aids testing the 'diff_test' rule.

load("//rules:diff_test.bzl", "diff_test")

package(default_testonly = 1)

sh_test(
name = "diff_test_tests",
srcs = ["diff_test_tests.sh"],
data = [
"//rules:diff_test",
"//tests:unittest.bash",
],
deps = ["@bazel_tools//tools/bash/runfiles"],
)

diff_test(
name = "same_src_src",
file1 = "a.txt",
file2 = "aa.txt",
)

diff_test(
name = "same_src_gen",
file1 = "a.txt",
file2 = "a-gen.txt",
)

diff_test(
name = "same_gen_gen",
file1 = "a-gen.txt",
file2 = "aa-gen.txt",
)

genrule(
name = "gen",
outs = [
"a-gen.txt",
"aa-gen.txt",
],
cmd = "echo -n 'potato' > $(location a-gen.txt) && echo -n 'potato' > $(location aa-gen.txt)",
)
1 change: 1 addition & 0 deletions tests/diff_test/a.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
potato
1 change: 1 addition & 0 deletions tests/diff_test/aa.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
potato
1 change: 1 addition & 0 deletions tests/diff_test/b.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
foo
Loading