Skip to content

Commit

Permalink
Improve message when docs are out-of-date.
Browse files Browse the repository at this point in the history
Checking in generated build artifacts makes things harder for contributors. The get a red CI status but have to know to look at the definition of the diff_test target to discover there's a command to auto-update. Instead we just print that command on failure.

The failure now looks like this:

```
$ bazel test //doc:all
INFO: Analyzed 9 targets (0 packages loaded, 0 targets configured).
INFO: Found 9 test targets...
FAIL: //doc:check_ios.doc (see /home/alexeagle/.cache/bazel/_bazel_alexeagle/32c0f21b6a1d2ed917381b593d4bcb34/execroot/build_bazel_rules_apple/bazel-out/k8-fastbuild/testlogs/doc/check_ios.doc/test.log)
INFO: From Testing //doc:check_ios.doc:
==================== Test output for //doc:check_ios.doc:
393c393
< | <a id="ios_sticker_pack_extension-entitlements"></a>entitlements |  The entitlements file required for device builds of this target. If absent, the default entitlements from the provisioning profile will be used.<br><br>The following variables are substituted in the entitlements file: <code>$(CFBundleIdentifier)</code> with the bundle ID of the application and <code>$(AppIdentifierPrefix)</code> with the value of the <code>ApplicationIdentifierPrefix</code> key from the target's provisioning profile.   | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
---
> | <a id="ios_sticker_pack_extension-entitlements"></a>entitlements |  The entitlements file required for device builds of this target. If absent, the default entitlements from the provisioning profile will be used.<br><br>The following variables are substituted in the entitlements file: <code>$(CFBundleIdentifier)</code> with the bundle ID of the application and <code>$(AppIdentifierPrefix)</code> with the value of the <code>ApplicationIdentifierPrefixe</code> key from the target's provisioning profile.   | <a href="https://bazel.build/docs/build-ref.html#labels">Label</a> | optional | None |
FAIL: files "doc/ios.doc.md" and "doc/rules-ios.md" differ.
Please update the docs by running
    bazel run //doc:update
================================================================================
INFO: Elapsed time: 0.067s, Critical Path: 0.02s
```
  • Loading branch information
alexeagle committed Jun 22, 2021
1 parent 0a2c39c commit 5f98726
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 2 deletions.
2 changes: 2 additions & 0 deletions .bazelci/update_workspace_to_deps_heads.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")\
\
git_repository(\
\ name = "bazel_skylib",\
\ patches = ["@build_bazel_rules_apple//doc:bazel-skylib.pr307.patch"],\
\ patch_args = ["-p1"],\
\ remote = "https://github.com/bazelbuild/bazel-skylib.git",\
\ branch = "main",\
)\
Expand Down
2 changes: 2 additions & 0 deletions apple/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ def apple_rules_dependencies(ignore_version_differences = False):
_maybe(
http_archive,
name = "bazel_skylib",
patches = ["@build_bazel_rules_apple//doc:bazel-skylib.pr307.patch"],
patch_args = ["-p1"],
urls = [
"https://github.com/bazelbuild/bazel-skylib/releases/download/1.0.3/bazel-skylib-1.0.3.tar.gz",
],
Expand Down
7 changes: 5 additions & 2 deletions doc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ _DOC_SRCS = [
for file in _DOC_SRCS
]

# If this test fails, run
# bazel run //doc:update
# Help developers who get a red CI result by telling them how to fix it
_failure_message = "\nPlease update the docs by running\n bazel run //doc:update"

[
diff_test(
name = "check_" + file,
failure_message = _failure_message,
file1 = file + ".md",
file2 = "rules-%s.md" % file.replace(".doc", ""),
)
Expand All @@ -57,6 +59,7 @@ stardoc(

diff_test(
name = "check_providers",
failure_message = _failure_message,
file1 = ":providers_doc",
file2 = "providers.md",
)
Expand Down
50 changes: 50 additions & 0 deletions doc/bazel-skylib.pr307.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
commit 58ad64fb010ce54e4530f2d0a5426e17c6199dce
Author: Alex Eagle <eagle@post.harvard.edu>
Date: Sun Jun 20 10:39:19 2021 -0700

diff_test: add ability for caller to specify a message printed on failure.

Useful for the use case of a pair of .test and .update targets for checked-in golden files

diff --git a/rules/diff_test.bzl b/rules/diff_test.bzl
index 71faf40..acde2ea 100644
--- a/rules/diff_test.bzl
+++ b/rules/diff_test.bzl
@@ -58,7 +58,7 @@ if "!RF2!" equ "" (
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
+ echo>&2 FAIL: files "{file1}" and "{file2}" differ. {fail_msg}
exit /b 1
) else (
fc.exe /B "!RF1!" "!RF2!"
@@ -66,6 +66,7 @@ if %ERRORLEVEL% neq 0 (
)
)
""".format(
+ fail_msg = ctx.attr.failure_message,
file1 = _runfiles_path(ctx.file.file1),
file2 = _runfiles_path(ctx.file.file2),
),
@@ -95,10 +96,11 @@ else
exit 1
fi
if ! diff "$RF1" "$RF2"; then
- echo >&2 "FAIL: files \"{file1}\" and \"{file2}\" differ"
+ echo >&2 "FAIL: files \"{file1}\" and \"{file2}\" differ. {fail_msg}"
exit 1
fi
""".format(
+ fail_msg = ctx.attr.failure_message,
file1 = _runfiles_path(ctx.file.file1),
file2 = _runfiles_path(ctx.file.file2),
),
@@ -112,6 +114,7 @@ fi

_diff_test = rule(
attrs = {
+ "failure_message": attr.string(),
"file1": attr.label(
allow_single_file = True,
mandatory = True,

0 comments on commit 5f98726

Please sign in to comment.