Skip to content

Commit 3597eca

Browse files
committed
fix permission errors in coverage directory
1 parent 012d505 commit 3597eca

File tree

10 files changed

+124
-43
lines changed

10 files changed

+124
-43
lines changed

build/deps/build_deps.jsonc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@
5858
},
5959
{
6060
"name": "aspect_rules_js",
61-
"type": "bazel_dep"
61+
"type": "github_tarball",
62+
"owner": "anonrig",
63+
"repo": "rules_js",
64+
"branch": "yagiz/fix-postprocessing",
65+
"freeze_commit": "93f17a3810649c95f658d85ff277fe963d1a20f8"
6266
},
6367
{
6468
"name": "aspect_rules_ts",

build/deps/gen/build_deps.MODULE.bazel

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,12 @@ bazel_dep(name = "aspect_rules_esbuild", version = "0.25.0")
1818

1919
# aspect_rules_js
2020
bazel_dep(name = "aspect_rules_js", version = "2.8.3")
21+
archive_override(
22+
module_name = "aspect_rules_js",
23+
integrity = "sha256-+3akxsGz4S5NKjTEPS/AafjsL+xU3GsOq2Nd3uVReps=",
24+
strip_prefix = "rules_js-93f17a3810649c95f658d85ff277fe963d1a20f8",
25+
urls = ["https://github.com/anonrig/rules_js/archive/93f17a3810649c95f658d85ff277fe963d1a20f8.tar.gz"],
26+
)
2127

2228
# aspect_rules_lint
2329
bazel_dep(name = "aspect_rules_lint", version = "1.13.0")

build/fixtures/BUILD.bazel

Lines changed: 0 additions & 1 deletion
This file was deleted.

build/fixtures/kj_test.sh

Lines changed: 0 additions & 3 deletions
This file was deleted.

build/kj_test.bzl

Lines changed: 106 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
load("@rules_cc//cc:cc_binary.bzl", "cc_binary")
2-
load("@rules_shell//shell:sh_test.bzl", "sh_test")
32

43
def kj_test(
54
src,
@@ -48,22 +47,118 @@ def kj_test(
4847
}),
4948
)
5049

51-
sh_test(
50+
_kj_test(
5251
name = test_name + "@",
5352
size = size,
54-
srcs = ["//build/fixtures:kj_test.sh"],
55-
data = [cross_alias] + data,
56-
args = ["$(location " + cross_alias + ")"],
53+
binary = cross_alias,
54+
data = data,
5755
tags = tags,
5856
)
5957

60-
# Tagged with no-coverage to reduce coverage CI time
61-
sh_test(
58+
_kj_test(
6259
name = test_name + "@all-autogates",
6360
size = size,
6461
env = {"WORKERD_ALL_AUTOGATES": "1"},
65-
srcs = ["//build/fixtures:kj_test.sh"],
66-
data = [cross_alias] + data,
67-
args = ["$(location " + cross_alias + ")"],
68-
tags = tags + ["no-coverage"],
62+
binary = cross_alias,
63+
data = data,
64+
tags = tags,
6965
)
66+
67+
# Shell template for kj_test - sets up coverage environment for the subprocess
68+
_SH_TEMPLATE = """#!/bin/sh
69+
set -e
70+
{env_exports}
71+
# Set up coverage for the test binary subprocess
72+
if [ -n "$COVERAGE_DIR" ]; then
73+
# Fix directory permissions for coverage post-processing
74+
# (Bazel may create COVERAGE_DIR with read-only permissions)
75+
chmod -R u+w "$COVERAGE_DIR" 2>/dev/null || true
76+
export LLVM_PROFILE_FILE="$COVERAGE_DIR/%p.profraw"
77+
export KJ_CLEAN_SHUTDOWN=1
78+
fi
79+
80+
exec {binary} "$@"
81+
"""
82+
83+
_BAT_TEMPLATE = """@echo off
84+
{env_exports}{binary} %*
85+
"""
86+
87+
def _kj_test_impl(ctx):
88+
is_windows = ctx.target_platform_has_constraint(ctx.attr._platforms_os_windows[platform_common.ConstraintValueInfo])
89+
90+
# Generate environment variable exports
91+
env_exports = ""
92+
for key, value in ctx.attr.env.items():
93+
if is_windows:
94+
env_exports += "set {}={}\n".format(key, value)
95+
else:
96+
env_exports += "export {}=\"{}\"\n".format(key, value)
97+
98+
if is_windows:
99+
executable = ctx.actions.declare_file("%s_kj_test.bat" % ctx.label.name)
100+
content = _BAT_TEMPLATE.format(binary = ctx.file.binary.short_path.replace("/", "\\"), env_exports = env_exports)
101+
else:
102+
executable = ctx.outputs.executable
103+
content = _SH_TEMPLATE.format(binary = ctx.file.binary.short_path, env_exports = env_exports)
104+
105+
ctx.actions.write(
106+
output = executable,
107+
content = content,
108+
is_executable = True,
109+
)
110+
111+
runfiles = ctx.runfiles(files = ctx.files.data + [ctx.file.binary])
112+
113+
# Merge the binary's runfiles
114+
default_runfiles = ctx.attr.binary[DefaultInfo].default_runfiles
115+
if default_runfiles:
116+
runfiles = runfiles.merge(default_runfiles)
117+
118+
# IMPORTANT: The binary must be listed in dependency_attributes to ensure
119+
# its transitive dependencies (all the C++ source files) are included in
120+
# coverage instrumentation. Without this, coverage data won't be collected.
121+
instrumented_files_info = coverage_common.instrumented_files_info(
122+
ctx,
123+
source_attributes = [],
124+
dependency_attributes = ["binary"],
125+
)
126+
127+
return [
128+
DefaultInfo(
129+
executable = executable,
130+
runfiles = runfiles,
131+
),
132+
instrumented_files_info,
133+
]
134+
135+
_kj_test = rule(
136+
implementation = _kj_test_impl,
137+
test = True,
138+
attrs = {
139+
# Implicit dependencies used by Bazel to generate coverage reports.
140+
"_lcov_merger": attr.label(
141+
default = configuration_field(fragment = "coverage", name = "output_generator"),
142+
executable = True,
143+
cfg = config.exec(exec_group = "test"),
144+
),
145+
"_collect_cc_coverage": attr.label(
146+
default = "//build/coverage:collect_cc_coverage",
147+
executable = True,
148+
cfg = config.exec(exec_group = "test"),
149+
),
150+
# The test binary to run
151+
"binary": attr.label(
152+
allow_single_file = True,
153+
executable = True,
154+
cfg = "target",
155+
mandatory = True,
156+
),
157+
# Additional data files needed by the test
158+
"data": attr.label_list(allow_files = True),
159+
# Environment variables to set when running the test
160+
"env": attr.string_dict(default = {}),
161+
# Reference to Windows platform for cross-platform support
162+
"_platforms_os_windows": attr.label(default = "@platforms//os:windows"),
163+
},
164+
)

build/wd_test.bzl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,9 @@ set -e
141141
142142
# Set up coverage for workerd subprocess
143143
if [ -n "$COVERAGE_DIR" ]; then
144+
# Fix directory permissions for coverage post-processing
145+
# (Bazel may create COVERAGE_DIR with read-only permissions)
146+
chmod -R u+w "$COVERAGE_DIR" 2>/dev/null || true
144147
export LLVM_PROFILE_FILE="$COVERAGE_DIR/%p.profraw"
145148
export KJ_CLEAN_SHUTDOWN=1
146149
fi
@@ -280,7 +283,7 @@ _wd_test = rule(
280283
cfg = config.exec(exec_group = "test"),
281284
),
282285
"_collect_cc_coverage": attr.label(
283-
default = "@bazel_tools//tools/test:collect_cc_coverage",
286+
default = "//build/coverage:collect_cc_coverage",
284287
executable = True,
285288
cfg = config.exec(exec_group = "test"),
286289
),

build/wd_ts_test.bzl

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,10 @@ def wd_ts_test(src, tsconfig_json, deps = [], eslintrc_json = None, composite =
1717
composite = composite,
1818
)
1919

20-
# TODO(soon): Remove 'no-coverage' tag once
21-
# https://github.com/aspect-build/rules_js/pull/2653 lands.
22-
# This is a workaround for a bug in aspect_rules_js with the
23-
# --experimental_split_coverage_postprocessing flag.
2420
js_test(
2521
name = name,
2622
entry_point = js_name(src),
2723
data = deps + [name + "@compile"],
28-
tags = ["no-arm64", "js-test", "no-coverage"],
24+
tags = ["no-arm64", "js-test"],
2925
**kwargs
3026
)
Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
load("@aspect_rules_js//js:defs.bzl", "js_test")
22

3-
# TODO(soon): Remove 'no-coverage' tag once
4-
# https://github.com/aspect-build/rules_js/pull/2653 lands.
5-
# This is a workaround for a bug in aspect_rules_js with the
6-
# --experimental_split_coverage_postprocessing flag.
73
js_test(
84
name = "inspector-test",
95
size = "large",
@@ -19,8 +15,5 @@ js_test(
1915
"WORKERD_BINARY": "$(rootpath //src/workerd/server:workerd)",
2016
"WORKERD_CONFIG": "$(rootpath :config.capnp)",
2117
},
22-
tags = [
23-
"js-test",
24-
"no-coverage",
25-
],
18+
tags = ["js-test"],
2619
)
Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
load("@aspect_rules_js//js:defs.bzl", "js_test")
22

3-
# TODO(soon): Remove 'no-coverage' tag once
4-
# https://github.com/aspect-build/rules_js/pull/2653 lands.
5-
# This is a workaround for a bug in aspect_rules_js with the
6-
# --experimental_split_coverage_postprocessing flag.
73
js_test(
84
name = "structured-logging-test",
95
data = [
@@ -15,5 +11,4 @@ js_test(
1511
"WORKERD_BINARY": "$(rootpath //src/workerd/server:workerd)",
1612
"WD_TEST_CONFIG": "$(rootpath :structured-logging-json.wd-test)",
1713
},
18-
tags = ["no-coverage"],
1914
)

tools/BUILD.bazel

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,14 @@ js_library(
1818
],
1919
)
2020

21-
# TODO(soon): Remove 'no-coverage' tag once
22-
# https://github.com/aspect-build/rules_js/pull/2653 lands.
23-
# This is a workaround for a bug in aspect_rules_js with the
24-
# --experimental_split_coverage_postprocessing flag.
2521
js_test(
2622
name = "custom-eslint-rules-test",
2723
size = "small",
2824
data = [
2925
":base-eslint",
3026
],
3127
entry_point = "custom-eslint-rules.test.mjs",
32-
tags = [
33-
"js-test",
34-
"no-coverage",
35-
],
28+
tags = ["js-test"],
3629
)
3730

3831
ts_config(

0 commit comments

Comments
 (0)