diff --git a/README.md b/README.md index 75a8930..27adedc 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ Code Ownership & Review Assignment Tool - GitHub CODEOWNERS but better [![Go Report Card](https://goreportcard.com/badge/github.com/multimediallc/codeowners-plus)](https://goreportcard.com/report/github.com/multimediallc/codeowners-plus?kill_cache=1) [![Tests](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml/badge.svg)](https://github.com/multimediallc/codeowners-plus/actions/workflows/go.yml) -![Coverage](https://img.shields.io/badge/Coverage-83.6%25-brightgreen) +![Coverage](https://img.shields.io/badge/Coverage-83.9%25-brightgreen) [![License](https://img.shields.io/badge/License-BSD%203--Clause-blue.svg)](https://opensource.org/licenses/BSD-3-Clause) [![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-2.1-4baaaa.svg)](CODE_OF_CONDUCT.md) @@ -25,6 +25,7 @@ Code Ownership & Review Assignment Tool - GitHub CODEOWNERS but better - [Quiet Mode](#quiet-mode) - [Hunk Filters](#hunk-filters) - [CLI Tool](#cli-tool) +- [JUnit Owners Action](#junit-owners-action) - [Contributing](#contributing) - [Future Features](#future-features) @@ -462,6 +463,92 @@ Available subcommands are: * `unowned` to check for unowned files * `owner` to check who owns a specific file or list of files * `validate` to check for typos in a `.codeowners` file +* `map` to generate a JSON ownership map of the entire repository +* `junit` to annotate JUnit XML test reports with code owners + +### Annotating test reports + +`junit` writes the owners of each test's source file onto its `` element: + +```bash +codeowners-cli junit --in-place --type pytest junit.xml +``` + +```xml + +``` + +This lets whatever consumes the report downstream — a test analytics service, a dashboard, a +flaky-test tracker — group results by the team that owns the test. + +Each testcase is traced back to a file in two ways. When the framework records a `file` +attribute (jest-junit's [`addFileAttribute`](https://github.com/jest-community/jest-junit#configuration), +among others) that path is used. Otherwise `classname` is read as a dotted module path, with +trailing segments trimmed until a real file is found, which is what pytest emits for both +module-level tests (`abuse.tests.test_abuse`) and class-based ones +(`abuse.tests.test_abuse.TestAbuse`). + +### Report types + +`--type` names the framework that produced the report, which selects the right strategy and, +just as importantly, skips the wrong one: + +`--type` is required, because there is no reliable way to tell the frameworks apart from the +report alone and guessing wrong misattributes tests. + +| Type | `file` attribute | `classname` as a path | Extensions tried | Writes `file` | +|------|------------------|-----------------------|------------------|---------------| +| `pytest` | yes | yes | `.py` | **yes** | +| `jest` | yes | **no** | — | no | + +`pytest` writes the resolved path back to `file` because pytest omits the attribute entirely +under its default `xunit2` family, so the write is purely additive. `jest` does not, because +overwriting a path the framework already set would change the meaning of a field its consumers +may rely on. + +`jest` also refuses to read `classname` as a path, because jest puts the text of the describe +block there. A block named something like `chatconnection.reconnectlimiter` looks exactly like a +module path and could otherwise resolve to an unrelated file. + +Reports that name files relative to a subdirectory rather than the repository root — as jest +does in a monorepo, where paths are relative to the package — need `--prefix`: + +```bash +codeowners-cli junit --in-place --type jest --prefix frontend/react frontend/junit-react.xml +``` + +Useful options: + +| Option | Purpose | +|--------|---------| +| `--in-place`, `-i` | Rewrite the report in place instead of writing to stdout | +| `--type`, `-t` | **Required.** Framework that produced the report: `pytest` or `jest` | +| `--prefix`, `-p` | Path prefix for reports that name files relative to a subdirectory | + +Testcases that cannot be resolved, and files with no owner, are left untouched. + +## JUnit Owners Action + +The `junit` subcommand is also packaged as an action, so annotating a report in CI does not +require installing the CLI yourself. Add it between the step that runs your tests and the step +that uploads the report: + +```yaml +- name: 'Annotate test results with code owners' + uses: multimediallc/codeowners-plus/actions/junit-owners@v1.11.0 + with: + path: junit.xml + type: pytest +``` + +| Input | Default | Purpose | +|-------|---------|---------| +| `path` | *required* | Report(s) to annotate; separate several with whitespace or commas | +| `type` | *required* | Framework that produced the report: `pytest` or `jest` | +| `root` | `.` | Path to the Git repository the reports belong to | +| `prefix` | `''` | Path prefix for reports that name files relative to a subdirectory | + ## Contributing diff --git a/actions/junit-owners/action.yml b/actions/junit-owners/action.yml new file mode 100644 index 0000000..ee513bf --- /dev/null +++ b/actions/junit-owners/action.yml @@ -0,0 +1,146 @@ +name: 'Codeowners Plus JUnit Owners' +description: 'Annotate JUnit XML test reports with the code owners of each test file' +inputs: + path: + description: 'JUnit XML report(s) to annotate. Separate multiple reports with whitespace or newlines.' + required: true + type: + description: 'Framework that produced the report: pytest or jest. Selects the resolution strategy, and determines whether the resolved path is written back to the `file` attribute.' + required: true + root: + description: 'Path to the Git repository the reports belong to' + required: false + default: '.' + prefix: + description: 'Path prefix to prepend to test file paths, for reports that name files relative to a subdirectory (for example a workspace root)' + required: false + default: '' + +runs: + using: 'composite' + steps: + - name: 'Resolve codeowners-cli binary' + id: resolve + shell: bash + env: + # The release tag this commit belongs to. Non-empty only in release + # commits: set by scripts/prepare-release.sh and cleared by + # scripts/post-release.sh. When set, the action downloads that + # release's prebuilt binary; when empty (any non-release ref) it + # builds from the checked-out source. + RELEASE_VERSION: '' + run: | + set -euo pipefail + # GITHUB_ACTION_PATH is this action's directory; the module root, which + # holds go.mod and scripts/, is two levels up. + ACTION_PATH=$(cd "${GITHUB_ACTION_PATH}/../.." && pwd) + + # The build cache has to be keyed on what is actually compiled. The ref + # alone is constant for a floating pin such as @main or @v1, so the + # cache would hit forever and upstream fixes would never be rebuilt. + # hashFiles() cannot do this: it only matches inside GITHUB_WORKSPACE, + # and a remote `uses:` puts the action under _actions/, outside it, + # where it returns an empty string and every revision shares one key. + # sha256sum is GNU coreutils (Linux); macOS runners only ship shasum. + if command -v sha256sum >/dev/null 2>&1; then + sum() { sha256sum "$@"; } + else + sum() { shasum -a 256 "$@"; } + fi + # Paths are relative to ACTION_PATH so the digest does not change with + # the checkout location, and go.mod/go.sum are included because they + # determine the toolchain and dependencies the binary is built from. + SRC_HASH=$( + cd "${ACTION_PATH}" && + { find tools/cli pkg -type f -name '*.go'; echo go.mod; echo go.sum; } | + LC_ALL=C sort | xargs sum | sum | cut -d' ' -f1 + ) + + { + echo "release-version=${RELEASE_VERSION}" + echo "bin=${RUNNER_TEMP:-/tmp}/codeowners-plus-cli/codeowners-cli" + echo "action-path=${ACTION_PATH}" + echo "src-hash=${SRC_HASH}" + } >>"$GITHUB_OUTPUT" + + # RELEASE_VERSION not set -> not a release: build from source (cached). + - name: 'Restore cached built binary' + id: buildcache + if: steps.resolve.outputs.release-version == '' + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ${{ steps.resolve.outputs.bin }} + key: codeowners-cli-build-${{ steps.resolve.outputs.src-hash }}-${{ runner.os }}-${{ runner.arch }} + + - name: 'Set up Go' + if: steps.resolve.outputs.release-version == '' && steps.buildcache.outputs.cache-hit != 'true' + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 + with: + go-version-file: ${{ steps.resolve.outputs.action-path }}/go.mod + cache-dependency-path: ${{ steps.resolve.outputs.action-path }}/go.sum + + - name: 'Build codeowners-cli from source' + if: steps.resolve.outputs.release-version == '' && steps.buildcache.outputs.cache-hit != 'true' + shell: bash + env: + ACTION_PATH: ${{ steps.resolve.outputs.action-path }} + BIN: ${{ steps.resolve.outputs.bin }} + run: | + set -euo pipefail + mkdir -p "$(dirname "${BIN}")" + cd "${ACTION_PATH}" + CGO_ENABLED=0 \ + go build -trimpath -buildvcs=false -ldflags="-s -w" -o "${BIN}" ./tools/cli + + # RELEASE_VERSION set -> a release: download + verify the prebuilt binary (cached). + - name: 'Restore cached release binary' + id: bincache + if: steps.resolve.outputs.release-version != '' + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: ${{ steps.resolve.outputs.bin }} + key: codeowners-cli-${{ steps.resolve.outputs.release-version }}-${{ runner.os }}-${{ runner.arch }} + + - name: 'Download codeowners-cli release binary' + if: steps.resolve.outputs.release-version != '' && steps.bincache.outputs.cache-hit != 'true' + shell: bash + env: + REPO: ${{ github.action_repository }} + ACTION_PATH: ${{ steps.resolve.outputs.action-path }} + TAG: ${{ steps.resolve.outputs.release-version }} + BIN: ${{ steps.resolve.outputs.bin }} + run: '"${ACTION_PATH}/scripts/install-cli.sh"' + + - name: 'Annotate JUnit reports' + shell: bash + env: + BIN: ${{ steps.resolve.outputs.bin }} + INPUT_PATH: ${{ inputs.path }} + INPUT_TYPE: ${{ inputs.type }} + INPUT_ROOT: ${{ inputs.root }} + INPUT_PREFIX: ${{ inputs.prefix }} + run: | + set -euo pipefail + + args=(junit --in-place --root "${INPUT_ROOT}" --type "${INPUT_TYPE}") + if [ -n "${INPUT_PREFIX}" ]; then + args+=(--prefix "${INPUT_PREFIX}") + fi + # `path` accepts several values; commas and whitespace are equivalent + # separators so that a YAML list, a multiline block or a + # comma-separated string all work. + # Split on whitespace only: globbing is disabled so that a path + # containing a wildcard is passed through as written rather than + # silently expanding, or vanishing when it matches nothing. + set -f + reports=() + for report in ${INPUT_PATH//,/ }; do + reports+=("${report}") + done + set +f + if [ "${#reports[@]}" -eq 0 ]; then + echo "Error: no reports given in 'path'." >&2 + exit 1 + fi + + "${BIN}" "${args[@]}" "${reports[@]}" diff --git a/scripts/install-cli.sh b/scripts/install-cli.sh new file mode 100755 index 0000000..96575e0 --- /dev/null +++ b/scripts/install-cli.sh @@ -0,0 +1,101 @@ +#! /usr/bin/env bash +# Installs the prebuilt codeowners-cli binary for the current platform, +# verified against the release's checksums.txt. +# +# The companion script install-action.sh does the same for the GitHub Action +# binary. They are kept separate because the two release archives follow +# different goreleaser naming templates. +# +# Local use (all env vars optional): +# scripts/install-cli.sh # latest release -> ./codeowners-cli +# VERSION=v1.11.0 scripts/install-cli.sh # a specific release +# BIN=/usr/local/bin/codeowners-cli scripts/install-cli.sh +# +# Overrides: REPO, VERSION (or TAG), OS, ARCH, BIN. The junit-owners action +# passes REPO/TAG/BIN; OS and ARCH are detected here so the script is +# self-contained. + +set -eu + +REPO="${REPO:-multimediallc/codeowners-plus}" +BIN="${BIN:-./codeowners-cli}" +TAG="${TAG:-${VERSION:-}}" + +# Detect OS unless overridden. The CLI archive title-cases the goreleaser +# {{ .Os }} token, so these are capitalized. +OS="${OS:-}" +if [ -z "${OS}" ]; then + case "$(uname -s)" in + Linux) OS="Linux" ;; + Darwin) OS="Darwin" ;; + *) + echo "Error: unsupported OS '$(uname -s)' (supported: Linux, Darwin)." >&2 + exit 1 + ;; + esac +fi + +# Detect ARCH unless overridden. The CLI archive spells amd64 as x86_64. +ARCH="${ARCH:-}" +if [ -z "${ARCH}" ]; then + case "$(uname -m)" in + x86_64 | amd64) ARCH="x86_64" ;; + arm64 | aarch64) ARCH="arm64" ;; + *) + echo "Error: unsupported arch '$(uname -m)' (supported: x86_64, arm64)." >&2 + exit 1 + ;; + esac +fi + +# Default to the latest release when no version was requested. +if [ -z "${TAG}" ]; then + TAG="$(curl -fsSL "https://api.github.com/repos/${REPO}/releases/latest" \ + | awk -F'"' '/"tag_name":/ {print $4; exit}')" + if [ -z "${TAG}" ]; then + echo "Error: could not determine the latest release of ${REPO}." >&2 + exit 1 + fi +fi + +# The CLI archive embeds the version without its leading "v". +asset="codeowners-cli_${TAG#v}_${OS}_${ARCH}.tar.gz" +binname="codeowners-cli" +base="https://github.com/${REPO}/releases/download/${TAG}" +tmp="$(mktemp -d)" +trap 'rm -rf "${tmp}"' EXIT + +echo "Downloading ${asset} from ${REPO} release ${TAG}" >&2 +curl -fsSL --retry 3 -o "${tmp}/${asset}" "${base}/${asset}" +curl -fsSL --retry 3 -o "${tmp}/checksums.txt" "${base}/checksums.txt" + +echo "Verifying ${asset} against checksums.txt" >&2 +expected="$(awk -v a="${asset}" '$2 == a {print $1}' "${tmp}/checksums.txt")" +if [ -z "${expected}" ]; then + echo "Error: ${asset} not found in checksums.txt" >&2 + exit 1 +fi +# Guard against a malformed digest: ' -c' treats an improperly +# formatted line as a skipped (passing) entry rather than a failure. +if ! printf '%s' "${expected}" | grep -Eq '^[0-9a-f]{64}$'; then + echo "Error: invalid checksum for ${asset} in checksums.txt" >&2 + exit 1 +fi +# sha256sum is GNU coreutils (Linux); macOS only ships shasum. +if command -v sha256sum >/dev/null 2>&1; then + verify=(sha256sum -c -) +else + verify=(shasum -a 256 -c -) +fi +if ! echo "${expected} ${tmp}/${asset}" | "${verify[@]}"; then + echo "Error: downloaded ${asset} does not match its release checksum" >&2 + exit 1 +fi + +echo "Extracting ${binname} from ${asset}" >&2 +tar -xzf "${tmp}/${asset}" -C "${tmp}" "${binname}" + +mkdir -p "$(dirname "${BIN}")" +mv "${tmp}/${binname}" "${BIN}" +chmod +x "${BIN}" +echo "Installed ${binname} ${TAG} to ${BIN}" >&2 diff --git a/scripts/post-release.sh b/scripts/post-release.sh index 9061f37..213b747 100755 --- a/scripts/post-release.sh +++ b/scripts/post-release.sh @@ -2,7 +2,7 @@ set -eu -ACTIONS_FILE="action.yml" +ACTIONS_FILES=("action.yml" "actions/junit-owners/action.yml") CLI_TOOL_FILE="tools/cli/main.go" README_FILE="README.md" @@ -39,25 +39,27 @@ else git checkout -b "${BRANCH_NAME}" fi -echo "Updating ${ACTIONS_FILE}, ${CLI_TOOL_FILE}, and ${README_FILE}..." +echo "Updating ${ACTIONS_FILES[*]}, ${CLI_TOOL_FILE}, and ${README_FILE}..." # sed -i works differently on macOS and Linux. # For GNU sed (Linux), -i without an argument is fine. # For BSD sed (macOS), -i requires an argument (even if empty string for no backup). if sed --version 2>/dev/null | grep -q GNU; then # GNU sed - sed -i "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: ''|g" "${ACTIONS_FILE}" + sed -i "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: ''|g" "${ACTIONS_FILES[@]}" sed -i "s|Version: .*|Version: \"${DEV_TAG}\",|g" "${CLI_TOOL_FILE}" sed -i "s|codeowners-plus@.*|codeowners-plus@${VERSION_TAG}|g" "${README_FILE}" + sed -i "s|codeowners-plus/actions/\([a-z-]*\)@.*|codeowners-plus/actions/\1@${VERSION_TAG}|g" "${README_FILE}" else # BSD sed (macOS) - sed -i '' "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: ''|g" "${ACTIONS_FILE}" + sed -i '' "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: ''|g" "${ACTIONS_FILES[@]}" sed -i '' "s|Version: .*|Version: \"${DEV_TAG}\",|g" "${CLI_TOOL_FILE}" sed -i '' "s|codeowners-plus@.*|codeowners-plus@${VERSION_TAG}|g" "${README_FILE}" + sed -i '' "s|codeowners-plus/actions/\([a-z-]*\)@.*|codeowners-plus/actions/\1@${VERSION_TAG}|g" "${README_FILE}" fi gofmt -w tools/cli -echo "${ACTIONS_FILE}, ${CLI_TOOL_FILE}, and ${README_FILE} updated." +echo "${ACTIONS_FILES[*]}, ${CLI_TOOL_FILE}, and ${README_FILE} updated." echo "Committing changes..." -git add "${ACTIONS_FILE}" "${CLI_TOOL_FILE}" "${README_FILE}" +git add "${ACTIONS_FILES[@]}" "${CLI_TOOL_FILE}" "${README_FILE}" git commit -m "${VERSION_TAG}" echo "--- Post release process completed successfully! ---" diff --git a/scripts/prepare-release.sh b/scripts/prepare-release.sh index 953e78d..aec5849 100755 --- a/scripts/prepare-release.sh +++ b/scripts/prepare-release.sh @@ -2,7 +2,7 @@ set -eu -ACTIONS_FILE="action.yml" +ACTIONS_FILES=("action.yml" "actions/junit-owners/action.yml") CLI_TOOL_FILE="tools/cli/main.go" README_FILE="README.md" @@ -11,7 +11,7 @@ function usage() { echo "Example: $0 1.2.3" echo " This script will:" echo " 1. Create a new branch called 'release/v1.2.3'." - echo " 2. Update '${ACTIONS_FILE}', ${CLI_TOOL_FILE}, and ${README_FILE} to reference the new version." + echo " 2. Update ${ACTIONS_FILES[*]}, ${CLI_TOOL_FILE}, and ${README_FILE} to reference the new version." echo " 3. Commit the changes." echo " 4. Create a tag called 'v1.2.3'." exit 1 @@ -53,10 +53,12 @@ echo "--- Starting release process for version ${SEMANTIC_VERSION} ---" check_git_clean -if [ ! -f "${ACTIONS_FILE}" ]; then - echo "Error: ${ACTIONS_FILE} not found in the current directory. Make sure you are running this from the root." - exit 1 -fi +for actions_file in "${ACTIONS_FILES[@]}"; do + if [ ! -f "${actions_file}" ]; then + echo "Error: ${actions_file} not found. Make sure you are running this from the root." + exit 1 + fi +done echo "Creating branch '${BRANCH_NAME}'..." if git rev-parse --verify "${BRANCH_NAME}" >/dev/null 2>&1; then @@ -66,25 +68,27 @@ else git checkout -b "${BRANCH_NAME}" fi -echo "Updating ${ACTIONS_FILE}, ${CLI_TOOL_FILE}, and ${README_FILE} to replace 'latest' or old tag with '${VERSION_TAG}'..." +echo "Updating ${ACTIONS_FILES[*]}, ${CLI_TOOL_FILE}, and ${README_FILE} to replace 'latest' or old tag with '${VERSION_TAG}'..." # sed -i works differently on macOS and Linux. # For GNU sed (Linux), -i without an argument is fine. # For BSD sed (macOS), -i requires an argument (even if empty string for no backup). if sed --version 2>/dev/null | grep -q GNU; then # GNU sed - sed -i "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: '${VERSION_TAG}'|g" "${ACTIONS_FILE}" + sed -i "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: '${VERSION_TAG}'|g" "${ACTIONS_FILES[@]}" sed -i "s|Version: .*|Version: \"${VERSION_TAG}\",|g" "${CLI_TOOL_FILE}" sed -i "s|codeowners-plus@.*|codeowners-plus@${VERSION_TAG}|g" "${README_FILE}" + sed -i "s|codeowners-plus/actions/\([a-z-]*\)@.*|codeowners-plus/actions/\1@${VERSION_TAG}|g" "${README_FILE}" else # BSD sed (macOS) - sed -i '' "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: '${VERSION_TAG}'|g" "${ACTIONS_FILE}" + sed -i '' "s|RELEASE_VERSION: '.*'|RELEASE_VERSION: '${VERSION_TAG}'|g" "${ACTIONS_FILES[@]}" sed -i '' "s|Version: .*|Version: \"${VERSION_TAG}\",|g" "${CLI_TOOL_FILE}" sed -i '' "s|codeowners-plus@.*|codeowners-plus@${VERSION_TAG}|g" "${README_FILE}" + sed -i '' "s|codeowners-plus/actions/\([a-z-]*\)@.*|codeowners-plus/actions/\1@${VERSION_TAG}|g" "${README_FILE}" fi gofmt -w tools/cli -echo "${ACTIONS_FILE}, ${CLI_TOOL_FILE}, and ${README_FILE} updated." +echo "${ACTIONS_FILES[*]}, ${CLI_TOOL_FILE}, and ${README_FILE} updated." echo "Committing changes..." -git add "${ACTIONS_FILE}" "${CLI_TOOL_FILE}" "${README_FILE}" +git add "${ACTIONS_FILES[@]}" "${CLI_TOOL_FILE}" "${README_FILE}" git commit -m "${VERSION_TAG}" echo "Creating tag '${VERSION_TAG}'..." diff --git a/tools/cli/junit.go b/tools/cli/junit.go new file mode 100644 index 0000000..3c876c8 --- /dev/null +++ b/tools/cli/junit.go @@ -0,0 +1,377 @@ +package main + +import ( + "bytes" + "encoding/xml" + "fmt" + "io" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "unicode" + "unicode/utf8" + + "github.com/multimediallc/codeowners-plus/pkg/codeowners" + f "github.com/multimediallc/codeowners-plus/pkg/functional" +) + +// ReportType names the framework that produced a report. Frameworks differ in +// how they identify the file behind a test, so knowing the producer lets the +// right strategy be used and, just as importantly, the wrong one be skipped. +type ReportType string + +const ( + // TypePytest reads `classname` as a dotted module path; pytest omits the + // `file` attribute under its default xunit2 family. + TypePytest ReportType = "pytest" + // TypeJest reads the `file` attribute; jest's `classname` holds the text of + // the describe block, which is prose rather than a path. + TypeJest ReportType = "jest" +) + +var allowedReportTypes = []string{string(TypePytest), string(TypeJest)} + +func validateReportType(reportType string) (ReportType, error) { + if !slices.Contains(allowedReportTypes, reportType) { + return "", fmt.Errorf("invalid type %s. Must be one of %s", reportType, strings.Join(allowedReportTypes, ", ")) + } + return ReportType(reportType), nil +} + +// writesFile reports whether the resolved path is written back to the `file` +// attribute. Only pytest does: it has no `file` attribute to begin with, so the +// write is purely additive, whereas overwriting one a framework already set +// changes the meaning of a field its consumers may rely on. +func (t ReportType) writesFile() bool { + return t == TypePytest +} + +// extensions returns the file extensions to try when reading `classname` as a +// dotted module path. It is empty for types that never read it that way. +func (t ReportType) extensions() []string { + if !t.usesClassname() { + return nil + } + return []string{".py"} +} + +// usesClassname reports whether `classname` may be read as a dotted module +// path. Doing so for jest would be actively harmful: a describe block named +// something like "chatconnection.reconnectlimiter" looks exactly like a module +// path and could resolve to an unrelated file. +func (t ReportType) usesClassname() bool { + return t != TypeJest +} + +const ownerSeparator = "," + +const ( + ownersAttr = "codeowners" + ownersCountAttr = ownersAttr + "Count" +) + +type junitOpts struct { + root string + prefix string + reportType ReportType + inPlace bool +} + +type fileResolver struct { + root string + prefix string + reportType ReportType + exts []string + cache map[string]bool +} + +func newFileResolver(root, prefix string, reportType ReportType) *fileResolver { + return &fileResolver{ + root: root, + prefix: prefix, + reportType: reportType, + exts: reportType.extensions(), + cache: make(map[string]bool), + } +} + +func (r *fileResolver) exists(rel string) bool { + if rel == "" || strings.HasPrefix(rel, "../") { + return false + } + if found, ok := r.cache[rel]; ok { + return found + } + stat, err := os.Stat(filepath.Join(r.root, rel)) + found := err == nil && !stat.IsDir() + r.cache[rel] = found + return found +} + +// candidates returns the repo-relative paths to try for a path taken from a +// report, most-specific first. A report may express paths relative to a +// subdirectory (jest names files relative to its own root), so the prefix is +// tried first, then the path as given, which keeps mixed reports working. +func (r *fileResolver) candidates(path string) []string { + if path == "" { + return nil + } + if filepath.IsAbs(path) { + rel, err := filepath.Rel(r.root, path) + if err != nil { + return nil + } + return []string{filepath.ToSlash(rel)} + } + clean := filepath.ToSlash(filepath.Clean(path)) + if r.prefix == "" { + return []string{clean} + } + return []string{filepath.ToSlash(filepath.Join(r.prefix, clean)), clean} +} + +// resolve locates the source file for a testcase, using its `file` attribute +// when the framework provides one (jest-junit's addFileAttribute, among +// others) and otherwise, where the report type allows it, reading `classname` +// as a dotted module path. Trailing class segments in classname are trimmed ("abuse.tests.test_abuse.TestAbuse") +func (r *fileResolver) resolve(file, classname string) string { + for _, candidate := range r.candidates(file) { + if r.exists(candidate) { + return candidate + } + } + + if classname == "" || !r.reportType.usesClassname() { + return "" + } + parts := strings.Split(classname, ".") + for { + for _, candidate := range r.candidates(strings.Join(parts, "/")) { + for _, ext := range r.exts { + if r.exists(candidate + ext) { + return candidate + ext + } + } + } + // Only class segments may be trimmed. Trimming a module segment would + // walk up into the enclosing package, where an unrelated file of the + // same name would have its owners stamped onto this test. + if len(parts) < 2 || !isClassSegment(parts[len(parts)-1]) { + return "" + } + parts = parts[:len(parts)-1] + } +} + +// isClassSegment reports whether a dotted-path segment looks like a test class +// rather than a module. Modules are lower case by convention (PEP 8) and +// pytest only collects classes matching its `python_classes` prefix, which is +// capitalised by default. +func isClassSegment(segment string) bool { + first, _ := utf8.DecodeRuneInString(segment) + return unicode.IsUpper(first) +} + +func isUnqualified(a xml.Attr, name string) bool { + return a.Name.Space == "" && a.Name.Local == name +} + +func attrValue(attrs []xml.Attr, name string) string { + for _, a := range attrs { + if isUnqualified(a, name) { + return a.Value + } + } + return "" +} + +func removeAttrValue(attrs []xml.Attr, name string) []xml.Attr { + for i, a := range attrs { + if isUnqualified(a, name) { + return append(attrs[:i], attrs[i+1:]...) + } + } + return attrs +} + +func setAttrValue(attrs []xml.Attr, name, value string) []xml.Attr { + for i, a := range attrs { + if isUnqualified(a, name) { + attrs[i].Value = value + return attrs + } + } + return append(attrs, xml.Attr{Name: xml.Name{Local: name}, Value: value}) +} + +// collectTestFiles decodes a report and returns the resolved file for each +// , in document order, with "" for any that could not be resolved. +func collectTestFiles(raw []byte, r *fileResolver) ([]string, error) { + decoder := xml.NewDecoder(bytes.NewReader(raw)) + files := make([]string, 0) + for { + token, err := decoder.Token() + if err == io.EOF { + break + } + if err != nil { + return nil, err + } + start, ok := token.(xml.StartElement) + if !ok || start.Name.Local != "testcase" { + continue + } + files = append(files, r.resolve(attrValue(start.Attr, "file"), attrValue(start.Attr, "classname"))) + } + return files, nil +} + +// rewrite streams the report back out, adding ownership attributes to each +// . Tokens are copied through untouched, so formatting, comments and +// failure output survive the round trip. +func rewrite(raw []byte, files []string, owners map[string][]string, o junitOpts) ([]byte, int, error) { + out := &bytes.Buffer{} + decoder := xml.NewDecoder(bytes.NewReader(raw)) + encoder := xml.NewEncoder(out) + i, annotated := 0, 0 + for { + token, err := decoder.Token() + if err == io.EOF { + break + } + if err != nil { + return nil, 0, err + } + if start, ok := token.(xml.StartElement); ok && start.Name.Local == "testcase" { + // A report may already carry attributes from an earlier run. + // Clearing them first keeps re-annotation idempotent: a test whose + // file has since become unowned, or can no longer be resolved at + // all, must not be left attributed to its former owners. + start.Attr = removeAttrValue(start.Attr, ownersAttr) + start.Attr = removeAttrValue(start.Attr, ownersCountAttr) + + if file := files[i]; file != "" { + // The write is only ever additive: a path the framework set + // itself is left alone, since its consumers may rely on the + // root it is relative to. + if o.reportType.writesFile() && attrValue(start.Attr, "file") == "" { + start.Attr = setAttrValue(start.Attr, "file", file) + } + if fileOwners := owners[file]; len(fileOwners) > 0 { + start.Attr = setAttrValue(start.Attr, ownersAttr, strings.Join(fileOwners, ownerSeparator)) + start.Attr = setAttrValue(start.Attr, ownersCountAttr, strconv.Itoa(len(fileOwners))) + annotated++ + } + } + i++ + token = start + } + if err := encoder.EncodeToken(token); err != nil { + return nil, 0, err + } + } + if err := encoder.Close(); err != nil { + return nil, 0, err + } + return out.Bytes(), annotated, nil +} + +func annotateJUnit(paths []string, o junitOpts) error { + if repoStat, err := os.Lstat(o.root); err != nil || !repoStat.IsDir() { + return fmt.Errorf("root is not a directory: %s", o.root) + } + if gitStat, err := os.Stat(filepath.Join(o.root, ".git")); err != nil || !gitStat.IsDir() { + return fmt.Errorf("root is not a Git repository: %s", o.root) + } + if !o.inPlace && len(paths) > 1 { + return fmt.Errorf("writing to stdout supports a single report; use --in-place for %d reports", len(paths)) + } + + // A report may name its files absolutely, and those can only be made + // repo-relative against an absolute root. + root, err := filepath.Abs(o.root) + if err != nil { + return fmt.Errorf("error resolving root %s: %w", o.root, err) + } + o.root = root + + type report struct { + path string + raw []byte + files []string + } + + resolver := newFileResolver(o.root, o.prefix, o.reportType) + reports := make([]*report, 0, len(paths)) + resolved := make(map[string]struct{}) + total, unresolved := 0, 0 + + for _, path := range paths { + raw, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("error reading %s: %w", path, err) + } + // The encoder rejects an XML declaration that is not the first token, + // so a byte order mark or leading whitespace has to go before the + // report can be streamed back out. + raw = bytes.TrimLeft(bytes.TrimPrefix(raw, []byte("\xef\xbb\xbf")), " \t\r\n") + files, err := collectTestFiles(raw, resolver) + if err != nil { + return fmt.Errorf("error parsing %s: %w", path, err) + } + for _, file := range files { + total++ + if file == "" { + unresolved++ + continue + } + resolved[file] = struct{}{} + } + reports = append(reports, &report{path: path, raw: raw, files: files}) + } + + if total > 0 && len(resolved) == 0 { + return fmt.Errorf("no testcase could be traced back to a file in the repository (%d testcases, all unresolved); check --type and --prefix", total) + } + + testFiles := make([]string, 0, len(resolved)) + for file := range resolved { + testFiles = append(testFiles, file) + } + slices.Sort(testFiles) + + diffFiles := f.Map(testFiles, func(file string) codeowners.DiffFile { + return codeowners.DiffFile{FileName: file} + }) + ownersMap, err := codeowners.New(o.root, diffFiles, &codeowners.FilesystemReader{}, io.Discard) + if err != nil { + return fmt.Errorf("error reading codeowners config: %w", err) + } + fileToOwners := mapFilesToOwners(ownersMap) + + annotated := 0 + for _, r := range reports { + out, count, err := rewrite(r.raw, r.files, fileToOwners, o) + if err != nil { + return fmt.Errorf("error rewriting %s: %w", r.path, err) + } + annotated += count + if !o.inPlace { + fmt.Println(string(out)) + continue + } + mode := os.FileMode(0o644) + if stat, err := os.Stat(r.path); err == nil { + mode = stat.Mode().Perm() + } + if err := os.WriteFile(r.path, out, mode); err != nil { + return fmt.Errorf("error writing %s: %w", r.path, err) + } + } + + _, _ = fmt.Fprintf(os.Stderr, "codeowners: annotated %d of %d testcases (%d resolved, %d unresolved)\n", + annotated, total, total-unresolved, unresolved) + return nil +} diff --git a/tools/cli/junit_test.go b/tools/cli/junit_test.go new file mode 100644 index 0000000..284d5bb --- /dev/null +++ b/tools/cli/junit_test.go @@ -0,0 +1,760 @@ +package main + +import ( + "encoding/xml" + "os" + "path/filepath" + "slices" + "strings" + "testing" +) + +func writeReport(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "report.xml") + if err := os.WriteFile(path, []byte(content), 0644); err != nil { + t.Fatalf("Failed to write report: %v", err) + } + return path +} + +// testcaseAttrs reads a report back and returns the attributes of each +// , keyed by the test name. +func testcaseAttrs(t *testing.T, path string) map[string]map[string]string { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + decoder := xml.NewDecoder(strings.NewReader(string(raw))) + found := make(map[string]map[string]string) + for { + token, err := decoder.Token() + if err != nil { + break + } + start, ok := token.(xml.StartElement) + if !ok || start.Name.Local != "testcase" { + continue + } + attrs := make(map[string]string) + for _, a := range start.Attr { + attrs[a.Name.Local] = a.Value + } + found[attrs["name"]] = attrs + } + return found +} + +func defaultOpts(root string, reportType ReportType) junitOpts { + return junitOpts{ + root: root, + reportType: reportType, + inPlace: true, + } +} + +func TestValidateReportType(t *testing.T) { + tt := []struct { + name string + input string + expected ReportType + expectedErr bool + }{ + {name: "pytest", input: "pytest", expected: TypePytest}, + {name: "jest", input: "jest", expected: TypeJest}, + {name: "auto is no longer accepted", input: "auto", expectedErr: true}, + {name: "unknown framework", input: "mocha", expectedErr: true}, + {name: "empty", input: "", expectedErr: true}, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + got, err := validateReportType(tc.input) + if (err != nil) != tc.expectedErr { + t.Errorf("validateReportType() error = %v, expectedErr %v", err, tc.expectedErr) + return + } + if got != tc.expected { + t.Errorf("validateReportType() = %v, want %v", got, tc.expected) + } + }) + } +} + +func TestReportTypeDefaults(t *testing.T) { + tt := []struct { + reportType ReportType + writesFile bool + useClassname bool + exts []string + }{ + {reportType: TypePytest, writesFile: true, useClassname: true, exts: []string{".py"}}, + {reportType: TypeJest, writesFile: false, useClassname: false, exts: nil}, + } + + for _, tc := range tt { + t.Run(string(tc.reportType), func(t *testing.T) { + if got := tc.reportType.writesFile(); got != tc.writesFile { + t.Errorf("writesFile() = %v, want %v", got, tc.writesFile) + } + if got := tc.reportType.usesClassname(); got != tc.useClassname { + t.Errorf("usesClassname() = %v, want %v", got, tc.useClassname) + } + if got := tc.reportType.extensions(); !slices.Equal(got, tc.exts) { + t.Errorf("extensions() = %v, want %v", got, tc.exts) + } + }) + } +} + +func TestAnnotateJUnitJestDoesNotReadClassnameAsPath(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // "internal.util" is prose here, but it looks exactly like a module path + // and a real internal/util.go exists. A jest report must not resolve it. + report := writeReport(t, ` + + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + if got, ok := cases["describe_block_that_looks_like_a_path"]["codeowners"]; ok { + t.Errorf("classname should not resolve to a path for jest, got %q", got) + } + // The file attribute still resolves normally. + if got := cases["has_a_real_file"]["codeowners"]; got != "@frontend-team" { + t.Errorf("codeowners = %q, want %q", got, "@frontend-team") + } +} + +func TestAnnotateJUnitByFileAttribute(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + report := writeReport(t, ` + + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + if got := cases["renders"]["codeowners"]; got != "@frontend-team" { + t.Errorf("renders codeowners = %q, want %q", got, "@frontend-team") + } + if got := cases["renders"]["codeownersCount"]; got != "1" { + t.Errorf("renders codeownersCount = %q, want %q", got, "1") + } + if got := cases["helps"]["codeowners"]; got != "@backend-team,@security-team" { + t.Errorf("helps codeowners = %q, want %q", got, "@backend-team,@security-team") + } + if got := cases["helps"]["codeownersCount"]; got != "2" { + t.Errorf("helps codeownersCount = %q, want %q", got, "2") + } +} + +func TestAnnotateJUnitByClassname(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // A dotted classname is either the module itself or the module plus the + // test class, and both must resolve to the same file. + report := writeReport(t, ` + + + +`) + + if err := os.WriteFile(filepath.Join(testRepo, "internal", "util.py"), []byte("# python"), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypePytest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + for _, name := range []string{"module_level", "class_level"} { + if got := cases[name]["codeowners"]; got != "@backend-team,@security-team" { + t.Errorf("%s codeowners = %q, want %q", name, got, "@backend-team,@security-team") + } + } +} + +func TestAnnotateJUnitPrefix(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // The first names its file relative to the prefix, the second relative to + // the repo root; a mixed report must resolve both. + report := writeReport(t, ` + + + +`) + + opts := defaultOpts(testRepo, TypeJest) + opts.prefix = "frontend" + if err := annotateJUnit([]string{report}, opts); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + for _, name := range []string{"prefixed", "rooted"} { + if got := cases[name]["codeowners"]; got != "@frontend-team" { + t.Errorf("%s codeowners = %q, want %q", name, got, "@frontend-team") + } + } +} + +func TestAnnotateJUnitPytestWritesFile(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + report := writeReport(t, ` + + + +`) + + if err := os.WriteFile(filepath.Join(testRepo, "internal", "util.py"), []byte("# python"), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypePytest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + for _, name := range []string{"from_classname", "from_class"} { + if got := cases[name]["file"]; got != "internal/util.py" { + t.Errorf("%s file = %q, want %q", name, got, "internal/util.py") + } + } +} + +func TestAnnotateJUnitLeavesUnresolvedTestcasesAlone(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + report := writeReport(t, ` + + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypePytest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + if _, ok := cases["missing"]["codeowners"]; ok { + t.Error("unresolvable testcase should not be annotated") + } + // The file resolves but has no owner, so there is nothing to write. + if _, ok := cases["unowned"]["codeowners"]; ok { + t.Error("unowned testcase should not be annotated") + } +} + +func TestAnnotateJUnitPreservesReportContent(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + report := writeReport(t, ` + + + +stack <trace> here + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + raw, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + out := string(raw) + for _, expected := range []string{ + ``, + ``, + `stack <trace> here`, + `tests="1"`, + `codeowners="@frontend-team"`, + } { + if !strings.Contains(out, expected) { + t.Errorf("annotated report missing %q\ngot: %s", expected, out) + } + } +} + +func TestAnnotateJUnitMultipleReportsShareOneLookup(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + first := writeReport(t, ` +`) + second := writeReport(t, ` +`) + + if err := annotateJUnit([]string{first, second}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + if got := testcaseAttrs(t, first)["one"]["codeowners"]; got != "@frontend-team" { + t.Errorf("first report codeowners = %q, want %q", got, "@frontend-team") + } + if got := testcaseAttrs(t, second)["two"]["codeowners"]; got != "@backend-team,@security-team" { + t.Errorf("second report codeowners = %q, want %q", got, "@backend-team,@security-team") + } +} + +func TestAnnotateJUnitResolvesAbsoluteFileAttribute(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // A relative root must still resolve an absolute path in the report. + report := writeReport(t, ` + + +`) + + opts := defaultOpts(testRepo, TypeJest) + opts.root = testRepo + "/." + if err := annotateJUnit([]string{report}, opts); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + if got := testcaseAttrs(t, report)["absolute"]["codeowners"]; got != "@frontend-team" { + t.Errorf("codeowners = %q, want %q", got, "@frontend-team") + } +} + +func TestAnnotateJUnitDoesNotOverwriteExistingFileAttribute(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + if err := os.WriteFile(filepath.Join(testRepo, "internal", "util.py"), []byte("# python"), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + report := writeReport(t, ` + + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypePytest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + if got := cases["keeps"]["file"]; got != "internal/util.py" { + t.Errorf("existing file attribute = %q, want it untouched", got) + } + if got := cases["gains"]["file"]; got != "internal/util.py" { + t.Errorf("missing file attribute = %q, want it written", got) + } +} + +func TestAnnotateJUnitDoesNotTrimPastTheModule(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // internal/util.py does not exist, so the only candidates left after + // trimming are the unrelated internal/ package and a top-level internal.py. + if err := os.WriteFile(filepath.Join(testRepo, "internal.py"), []byte("# unrelated"), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + report := writeReport(t, ` + + +`) + + before, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypePytest)); err == nil { + t.Fatal("annotateJUnit() should fail when nothing resolves") + } + if _, ok := testcaseAttrs(t, report)["module_is_missing"]["codeowners"]; ok { + t.Error("a missing module must not be attributed to an unrelated file") + } + after, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + if string(before) != string(after) { + t.Error("a report must be left untouched when nothing resolves") + } +} + +func TestAnnotateJUnitStripsByteOrderMark(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + report := writeReport(t, "\xef\xbb\xbf"+` + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + if got := testcaseAttrs(t, report)["bom"]["codeowners"]; got != "@frontend-team" { + t.Errorf("codeowners = %q, want %q", got, "@frontend-team") + } +} + +func TestAnnotateJUnitCountsOnlyWhatItWrote(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // unowned/file.txt resolves but has no owner, so nothing is written for it. + report := writeReport(t, ` + + + +`) + + raw, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + resolved := []string{"frontend/app.js", "unowned/file.txt"} + owners := map[string][]string{"frontend/app.js": {"@frontend-team"}} + _, annotated, err := rewrite(raw, resolved, owners, defaultOpts(testRepo, TypeJest)) + if err != nil { + t.Fatalf("rewrite() error = %v", err) + } + if annotated != 1 { + t.Errorf("rewrite() annotated = %d, want 1 (the unowned file carries no attribute)", annotated) + } +} + +func TestAnnotateJUnitRejectsMultipleReportsToStdout(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + first := writeReport(t, ``) + second := writeReport(t, ``) + + opts := defaultOpts(testRepo, TypeJest) + opts.inPlace = false + if err := annotateJUnit([]string{first, second}, opts); err == nil { + t.Error("annotateJUnit() should refuse several reports without --in-place") + } +} + +func TestIsClassSegment(t *testing.T) { + tt := []struct { + name string + input string + expected bool + }{ + {name: "test class", input: "TestFoo", expected: true}, + {name: "module", input: "test_foo", expected: false}, + {name: "empty", input: "", expected: false}, + // The leading byte of a multi-byte character is itself a code point + // that unicode.IsUpper reports as upper case for much of Latin-1, so + // these must be decoded rather than indexed. + {name: "lower case accented module", input: "\u00f3micron", expected: false}, + {name: "lower case umlaut module", input: "\u00fcber", expected: false}, + {name: "lower case cyrillic module", input: "\u0442\u0435\u0441\u0442", expected: false}, + {name: "upper case accented class", input: "\u00d3micron", expected: true}, + {name: "upper case cyrillic class", input: "\u0422\u0435\u0441\u0442", expected: true}, + {name: "caseless script", input: "\u65e5\u672c\u8a9e", expected: false}, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + if got := isClassSegment(tc.input); got != tc.expected { + t.Errorf("isClassSegment(%q) = %v, want %v", tc.input, got, tc.expected) + } + }) + } +} + +func TestAnnotateJUnitDoesNotTrimAccentedModule(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // "\u00f3micron" is a package, not a class, so a missing module beneath it must + // not fall through and take the owners of \u00f3micron.py. + if err := os.MkdirAll(filepath.Join(testRepo, "\u00f3micron"), 0755); err != nil { + t.Fatalf("Failed to create directory: %v", err) + } + if err := os.WriteFile(filepath.Join(testRepo, "\u00f3micron.py"), []byte("# unrelated"), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + report := writeReport(t, ` + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypePytest)); err == nil { + t.Fatal("annotateJUnit() should fail when nothing resolves") + } + if _, ok := testcaseAttrs(t, report)["t"]["codeowners"]; ok { + t.Error("an accented package segment must not be trimmed as if it were a class") + } +} + +func TestAnnotateJUnitLeavesReportIntactWhenNothingResolves(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + report := writeReport(t, ` + + +`) + before, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + + // The wrong prefix resolves nothing. The run must abort before writing, + // leaving the annotation an earlier correct run produced. + opts := defaultOpts(testRepo, TypeJest) + opts.prefix = "wrong" + if err := annotateJUnit([]string{report}, opts); err == nil { + t.Fatal("annotateJUnit() should fail when a bad prefix resolves nothing") + } + + after, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + if string(before) != string(after) { + t.Errorf("a failed run must not rewrite the report\nbefore: %s\nafter: %s", before, after) + } + if got := testcaseAttrs(t, report)["annotated"]["codeowners"]; got != "@frontend-team" { + t.Errorf("existing annotation was lost: codeowners = %q", got) + } +} + +func TestIsUnqualified(t *testing.T) { + tt := []struct { + name string + attr xml.Attr + expected bool + }{ + { + name: "plain attribute", + attr: xml.Attr{Name: xml.Name{Local: "codeowners"}}, + expected: true, + }, + { + name: "prefixed attribute", + attr: xml.Attr{Name: xml.Name{Space: "vendor", Local: "codeowners"}}, + expected: false, + }, + { + name: "namespaced attribute", + attr: xml.Attr{Name: xml.Name{Space: "http://example.com/v", Local: "codeowners"}}, + expected: false, + }, + { + name: "different name", + attr: xml.Attr{Name: xml.Name{Local: "classname"}}, + expected: false, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + if got := isUnqualified(tc.attr, "codeowners"); got != tc.expected { + t.Errorf("isUnqualified(%v, %q) = %v, want %v", tc.attr.Name, "codeowners", got, tc.expected) + } + }) + } +} + +func TestAnnotateJUnitPreservesNamespacedAttributes(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // Someone else's vendor:codeowners must survive untouched, both where this + // tool writes its own attribute and where it clears a stale one. + report := writeReport(t, ` + + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + raw, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + + // The encoder rewrites namespace prefixes, so assert on the decoded + // attribute rather than on the serialised form. + type attrs struct{ plain, namespaced string } + found := make(map[string]attrs) + decoder := xml.NewDecoder(strings.NewReader(string(raw))) + for { + token, err := decoder.Token() + if err != nil { + break + } + start, ok := token.(xml.StartElement) + if !ok || start.Name.Local != "testcase" { + continue + } + var name string + var got attrs + for _, a := range start.Attr { + switch { + case isUnqualified(a, "name"): + name = a.Value + case isUnqualified(a, "codeowners"): + got.plain = a.Value + case a.Name.Space != "" && a.Name.Local == "codeowners": + got.namespaced = a.Value + } + } + found[name] = got + } + + if got := found["owned"].namespaced; got != "@them" { + t.Errorf("owned: vendor:codeowners = %q, want %q", got, "@them") + } + if got := found["owned"].plain; got != "@frontend-team" { + t.Errorf("owned: codeowners = %q, want %q", got, "@frontend-team") + } + // The unowned testcase loses its own stale attribute but keeps the vendor one. + if got := found["unowned"].namespaced; got != "@them" { + t.Errorf("unowned: vendor:codeowners = %q, want it preserved", got) + } + if got := found["unowned"].plain; got != "" { + t.Errorf("unowned: codeowners = %q, want it cleared", got) + } +} + +func TestAnnotateJUnitClearsStaleAttributes(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + // Every testcase arrives already annotated by an earlier run. Only the + // first still has an owner; the second resolves to a file that no longer + // has one, and the third no longer resolves at all. + report := writeReport(t, ` + + + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("annotateJUnit() error = %v", err) + } + + cases := testcaseAttrs(t, report) + if got := cases["still_owned"]["codeowners"]; got != "@frontend-team" { + t.Errorf("still_owned codeowners = %q, want %q", got, "@frontend-team") + } + if got := cases["still_owned"]["codeownersCount"]; got != "1" { + t.Errorf("still_owned codeownersCount = %q, want %q", got, "1") + } + for _, name := range []string{"now_unowned", "now_unresolved"} { + if got, ok := cases[name]["codeowners"]; ok { + t.Errorf("%s kept a stale codeowners attribute: %q", name, got) + } + if got, ok := cases[name]["codeownersCount"]; ok { + t.Errorf("%s kept a stale codeownersCount attribute: %q", name, got) + } + } +} + +func TestAnnotateJUnitIsIdempotent(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + report := writeReport(t, ` + + +`) + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("first annotateJUnit() error = %v", err) + } + first, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + + if err := annotateJUnit([]string{report}, defaultOpts(testRepo, TypeJest)); err != nil { + t.Fatalf("second annotateJUnit() error = %v", err) + } + second, err := os.ReadFile(report) + if err != nil { + t.Fatalf("Failed to read report: %v", err) + } + + if string(first) != string(second) { + t.Errorf("re-annotating changed the report\nfirst: %s\nsecond: %s", first, second) + } +} + +func TestAnnotateJUnitErrors(t *testing.T) { + testRepo, cleanup := setupTestRepo(t) + defer cleanup() + + tt := []struct { + name string + paths []string + opts func(junitOpts) junitOpts + }{ + { + name: "root is not a directory", + paths: []string{writeReport(t, "")}, + opts: func(o junitOpts) junitOpts { + o.root = filepath.Join(testRepo, "main.go") + return o + }, + }, + { + name: "root is not a git repository", + paths: []string{writeReport(t, "")}, + opts: func(o junitOpts) junitOpts { + o.root = t.TempDir() + return o + }, + }, + { + name: "report does not exist", + paths: []string{filepath.Join(testRepo, "no-such-report.xml")}, + opts: func(o junitOpts) junitOpts { return o }, + }, + { + name: "report is not valid xml", + paths: []string{writeReport(t, "")}, + opts: func(o junitOpts) junitOpts { return o }, + }, + } + + for _, tc := range tt { + t.Run(tc.name, func(t *testing.T) { + if err := annotateJUnit(tc.paths, tc.opts(defaultOpts(testRepo, TypeJest))); err == nil { + t.Error("annotateJUnit() expected an error, got nil") + } + }) + } +} diff --git a/tools/cli/main.go b/tools/cli/main.go index a29440c..ca32b31 100644 --- a/tools/cli/main.go +++ b/tools/cli/main.go @@ -210,6 +210,62 @@ func main() { return generateOwnershipMap(repo, mapBy) }, }, + { + Name: "junit", + Aliases: []string{"j"}, + Usage: "Annotate JUnit XML reports with code owners", + UsageText: "codeowners-cli junit [options] [report2.xml]...\n or: cat reports.txt | codeowners-cli junit [options]", + Description: "Annotate JUnit XML reports with code owners. Each is traced back to the file that defines it and the owners of that file are written onto the element as an attribute, so that whatever consumes the report can group test results by ownership. Frameworks identify the file behind a test differently, so --type names the one that produced the report: it selects the right strategy and skips the wrong one.", + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "root", + Aliases: []string{"r", "repo"}, + Value: "./", + Usage: "Path to local Git repo", + Destination: &repo, + }, + &cli.StringFlag{ + Name: "type", + Aliases: []string{"t"}, + Required: true, + Usage: "Framework that produced the report. Allowed values are: pytest and jest", + }, + &cli.StringFlag{ + Name: "prefix", + Aliases: []string{"p"}, + Value: "", + Usage: "Path prefix to prepend to test file paths, for reports that name files relative to a subdirectory", + }, + &cli.BoolFlag{ + Name: "in-place", + Aliases: []string{"i"}, + Value: false, + Usage: "Rewrite the reports in place instead of writing to stdout", + }, + }, + Action: func(ctx context.Context, cmd *cli.Command) error { + targets, err := getTargets(cmd) + if err != nil { + return err + } + + if len(targets) == 0 { + return fmt.Errorf("no target reports provided (either as arguments or from stdin)") + } + + reportType, err := validateReportType(cmd.String("type")) + if err != nil { + return err + } + + return annotateJUnit(targets, junitOpts{ + root: repo, + prefix: cmd.String("prefix"), + reportType: reportType, + inPlace: cmd.Bool("in-place"), + }) + }, + }, }, }