Skip to content

CI: gofmt failures are masked and surface as confusing 'coverage.xml not found' #657

Description

@hdwhdw

Observed in build 1094908 (PR #643): the visible failure is ##[error]Path does not exist: .../sonic-gnmi/coverage.xml on the Publish integration coverage artifact step, but the real root cause is a gofmt formatting violation in many .go files. The formatting error happened inside the Run Integration Tests step, which was reported as succeeded.

Root cause chain

  1. The integration-test and memory-leak steps use this pattern:

    set -euo pipefail
    pushd sonic-gnmi
    make all && $(UNIT_TEST_FLAG) make check_*_junit
    popd

    Per Bash's own documentation, set -e is disabled for commands that are not the final command in an && list. So when make all fails on .formatcheck, the step does not abort. popd runs last, returns 0, and Azure DevOps reports the task as green. coverage.xml is never produced.

  2. The downstream Publish integration coverage artifact publish step has condition: always(), so it runs anyway and loudly fails with the misleading "Path does not exist" message — which is all a reviewer sees at the top of the results page.

  3. The .formatcheck Makefile target outputs all ~170 offending file names on a single line and shows no diff, so even when the real failure is identified it is hard to act on.

Proposed fix

Tracked by PR #656:

  • Add a dedicated Go format check (gofmt) step to the fast PureCIJob so formatting issues fail in seconds at the top of the page.
  • Replace pushd … && … popd with cd … ; cmd1 ; cmd2 in the integration and memory-leak steps so set -e actually aborts on first failure.
  • Change the coverage-artifact publish step to condition: succeeded() so it no longer emits a secondary misleading failure.
  • Improve the .formatcheck target output (per-file list + gofmt -d diff) and add make fmt / make fmt-check convenience targets.

Acceptance

  • A PR that introduces a formatting violation fails on the dedicated format-check step in PureCIJob within seconds, with a readable per-file diff in the log.
  • The integration-test and memory-leak steps fail immediately on the first real error and are reported as failed (not succeeded).
  • When upstream tests fail, the Publish integration coverage artifact step is skipped rather than failing with a confusing "coverage.xml not found".

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions