diff --git a/.github/workflows/pr-coverage.yml b/.github/workflows/pr-coverage.yml new file mode 100644 index 0000000..b3cbf7e --- /dev/null +++ b/.github/workflows/pr-coverage.yml @@ -0,0 +1,70 @@ +name: pr-code-coverage + +# Dogfoods this plugin on its own PRs: builds the plugin image from the PR +# source, computes coverage for only the lines changed in the PR, and posts a +# summary comment on the PR. + +on: + pull_request: + +permissions: + contents: read + pull-requests: write # required so the plugin can post the coverage comment + +jobs: + coverage: + runs-on: ubuntu-latest + # Fork PRs only get a read-only GITHUB_TOKEN (can't comment) and no secrets, + # so restrict to same-repo PRs to avoid a guaranteed failure on forks. + if: github.event.pull_request.head.repo.full_name == github.repository + steps: + - name: Check out the repo + uses: actions/checkout@v4 + with: + fetch-depth: 0 # need the base branch present to diff against it + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version: 1.26.3 + + - name: Generate coverage profile + run: go test -coverpkg=./... -coverprofile=coverage.txt ./... + + - name: Convert coverage to cobertura + # go run leaves no installed binary; boumenot emits as the + # absolute build dir (== github.workspace) and class filenames relative + # to the module root, which is what PARAMETER_SOURCE_DIRS matches against. + run: go run github.com/boumenot/gocover-cobertura@v1.5.0 < coverage.txt > coverage.xml + + - name: Build plugin image + # The published :latest image predates the public-github.com base-URL fix, + # so build from the PR source to test the exact code under review. + run: docker build -t pr-code-coverage:ci . + + - name: Report coverage on changed lines + env: + PARAMETER_COVERAGE_TYPE: cobertura + PARAMETER_COVERAGE_FILE: coverage.xml + # Must equal the cobertura path (the dir go test ran in). + PARAMETER_SOURCE_DIRS: ${{ github.workspace }} + # Omit PARAMETER_GH_API_BASE_URL -> defaults to https://api.github.com. + PARAMETER_GH_API_KEY: ${{ secrets.GITHUB_TOKEN }} + BUILD_PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number }} + REPOSITORY_ORG: ${{ github.repository_owner }} + REPOSITORY_NAME: ${{ github.event.repository.name }} + run: | + git fetch --no-tags origin "${{ github.base_ref }}" + git --no-pager diff --unified=0 "origin/${{ github.base_ref }}" -- '*.go' \ + | docker run --rm -i \ + -e PARAMETER_COVERAGE_TYPE \ + -e PARAMETER_COVERAGE_FILE \ + -e PARAMETER_SOURCE_DIRS \ + -e PARAMETER_GH_API_KEY \ + -e BUILD_PULL_REQUEST_NUMBER \ + -e REPOSITORY_ORG \ + -e REPOSITORY_NAME \ + -v "${{ github.workspace }}:${{ github.workspace }}" \ + -w "${{ github.workspace }}" \ + --entrypoint /plugin \ + pr-code-coverage:ci diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f5623d2..092e2f4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,17 +10,17 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out the repo - uses: actions/checkout@v2 + uses: actions/checkout@v4 - name: Docker Login - uses: docker/login-action@v1 + uses: docker/login-action@v3 with: username: ${{ secrets.DOCKERHUB_USERNAME }} password: ${{ secrets.DOCKERHUB_TOKEN }} - name: Docker Build and Push id: docker_build - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v6 with: push: true tags: | - target/pull-request-code-coverage:latest - target/pull-request-code-coverage:${{ github.event.release.tag_name }} \ No newline at end of file + pullrequestcc/pull-request-code-coverage:latest + pullrequestcc/pull-request-code-coverage:${{ github.event.release.tag_name }} \ No newline at end of file diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a781a06..ee04ecb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,12 +6,12 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set up Go - uses: actions/setup-go@v2 + uses: actions/setup-go@v5 with: - go-version: 1.17 + go-version: 1.26.3 - name: Build run: go build -v ./... diff --git a/.golangci.yml b/.golangci.yml index 28a8a3b..99185cd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,39 +1,35 @@ +version: "2" + linters: - disable-all: true + default: none enable: - bodyclose - - deadcode - - depguard - dogsled - errcheck - gochecknoinits - goconst - gocritic - gocyclo - - gofmt - - goimports - - golint - goprintffuncname - gosec - govet - ineffassign - - interfacer - - maligned - misspell - nakedret - rowserrcheck - - scopelint - staticcheck - - structcheck - - stylecheck - - typecheck - unconvert - unparam - unused - - varcheck -issues: - exclude-rules: - - path: _test\.go - linters: - - scopelint - - funlen \ No newline at end of file + exclusions: + rules: + - path: _test\.go + linters: + - funlen + - goconst + - gosec + +formatters: + enable: + - gofmt + - goimports diff --git a/Dockerfile b/Dockerfile index aa84fe8..5214a90 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.17-alpine as builder +FROM golang:1.26.3-alpine AS builder WORKDIR /go/src/github.com/target/pull-request-code-coverage COPY . . ENV GO111MODULE=on diff --git a/Makefile b/Makefile index e1b823d..0ab7549 100644 --- a/Makefile +++ b/Makefile @@ -21,9 +21,11 @@ check-gofmt: echo "Success - way to run gofmt!"; \ fi +GOLANGCI_LINT_VERSION=v2.12.2 + bin/golangci-lint: mkdir -p bin - wget -O- -nv https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.24.0 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/HEAD/install.sh | sh -s -- -b ./bin $(GOLANGCI_LINT_VERSION) .PHONY: lint lint: bin/golangci-lint diff --git a/README.md b/README.md index 4a67ed9..75e11cd 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,9 @@ this project. Once you have that jacoco file, you can pass that path to coverage source_dirs: - src/main/java - src/main/kotlin - gh_api_base_url: https://git.target.com + # omit for public github.com (defaults to https://api.github.com) + # for GitHub Enterprise, use the full API root including /api/v3 + gh_api_base_url: https://git.target.com/api/v3 module: some-sub-module secrets: - source: pull_request_api_key @@ -73,7 +75,9 @@ Once you have coverage.xml same can be passed as an input to plugin shown below coverage_file: coverage.xml source_dirs: - /vela/src/github.com/targetOSS/pull-request-code-coverage - gh_api_base_url: https://git.target.com + # omit for public github.com (defaults to https://api.github.com) + # for GitHub Enterprise, use the full API root including /api/v3 + gh_api_base_url: https://git.target.com/api/v3 secrets: - source: pull_request_api_key target: plugin_gh_api_key @@ -92,7 +96,7 @@ Once you have coverage.xml same can be passed as an input to plugin shown below # Development -This project needs go (>= 1.17) to be installed. Make sure you run +This project needs go (>= 1.26.3) to be installed. Make sure you run * make format * make lint diff --git a/go.mod b/go.mod index de194c4..78138dd 100644 --- a/go.mod +++ b/go.mod @@ -1,18 +1,17 @@ module github.com/target/pull-request-code-coverage -go 1.17 +go 1.26.3 require ( github.com/pkg/errors v0.9.1 - github.com/sirupsen/logrus v1.8.1 - github.com/stretchr/testify v1.7.0 - + github.com/sirupsen/logrus v1.9.4 + github.com/stretchr/testify v1.11.1 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/stretchr/objx v0.1.0 // indirect - golang.org/x/sys v0.0.0-20191026070338-33540a1f6037 // indirect - gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c // indirect + github.com/stretchr/objx v0.5.3 // indirect + golang.org/x/sys v0.44.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 2fe2451..4c19f8a 100644 --- a/go.sum +++ b/go.sum @@ -1,20 +1,18 @@ -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/sirupsen/logrus v1.8.1 h1:dJKuHgqk1NNQlqoA6BTlM1Wf9DOH3NBjQyu0h9+AZZE= -github.com/sirupsen/logrus v1.8.1/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= -github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= -github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -golang.org/x/sys v0.0.0-20191026070338-33540a1f6037 h1:YyJpGZS1sBuBCzLAR1VEpK193GlqGZbnPFnPV/5Rsb4= -golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +github.com/sirupsen/logrus v1.9.4 h1:TsZE7l11zFCLZnZ+teH4Umoq5BhEIfIzfRDZ1Uzql2w= +github.com/sirupsen/logrus v1.9.4/go.mod h1:ftWc9WdOfJ0a92nsE2jF5u5ZwH8Bv2zdeOC42RjbV2g= +github.com/stretchr/objx v0.5.3 h1:jmXUvGomnU1o3W/V5h2VEradbpJDwGrzugQQvL0POH4= +github.com/stretchr/objx v0.5.3/go.mod h1:rDQraq+vQZU7Fde9LOZLr8Tax6zZvy4kuNKF+QYS+U0= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +golang.org/x/sys v0.44.0 h1:ildZl3J4uzeKP07r2F++Op7E9B29JRUy+a27EibtBTQ= +golang.org/x/sys v0.44.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/plugin/coverage/cobertura/report.go b/internal/plugin/coverage/cobertura/report.go index 39051df..9d2e397 100644 --- a/internal/plugin/coverage/cobertura/report.go +++ b/internal/plugin/coverage/cobertura/report.go @@ -3,7 +3,6 @@ package cobertura import ( "encoding/xml" "io" - "io/ioutil" "log" "os" @@ -22,7 +21,7 @@ type DefaultLoader struct { func NewReportLoader(sourceDir string) *DefaultLoader { return &DefaultLoader{ - readAllFunc: ioutil.ReadAll, + readAllFunc: io.ReadAll, sourceDir: sourceDir, } } diff --git a/internal/plugin/coverage/jacoco/report.go b/internal/plugin/coverage/jacoco/report.go index a87e471..3d97b36 100644 --- a/internal/plugin/coverage/jacoco/report.go +++ b/internal/plugin/coverage/jacoco/report.go @@ -4,7 +4,6 @@ import ( "encoding/xml" "io" - "io/ioutil" "log" "os" @@ -20,12 +19,12 @@ type DefaultLoader struct { func NewReportLoader() *DefaultLoader { return &DefaultLoader{ - readAllFunc: ioutil.ReadAll, + readAllFunc: io.ReadAll, } } func (l *DefaultLoader) Load(coverageFile string) (coverage.Report, error) { - // nolint: gosec + // nolint: gosec // coverageFile is a user-supplied report path; opening it is the intended behavior xmlFile, openFileErr := os.Open(coverageFile) if openFileErr != nil { @@ -96,7 +95,7 @@ type SourceFile struct { func (f SourceFile) GetCoverageData(lineNumber int) (*domain.CoverageData, bool) { for _, l := range f.Lines { if l.LineNumber == lineNumber { - coverageData := l.CoverageData.toDomain() + coverageData := l.toDomain() return &coverageData, true } } diff --git a/internal/plugin/pluginhttp/client.go b/internal/plugin/pluginhttp/client.go index 260fae0..e024040 100644 --- a/internal/plugin/pluginhttp/client.go +++ b/internal/plugin/pluginhttp/client.go @@ -18,5 +18,6 @@ func (c *DefaultClient) NewRequest(method string, url string, body io.Reader) (* } func (c *DefaultClient) Do(request *http.Request) (*http.Response, error) { + // nolint: gosec // the request targets the user-configured GitHub API URL, which is the intended behavior return http.DefaultClient.Do(request) } diff --git a/internal/plugin/reporter/github_pr.go b/internal/plugin/reporter/github_pr.go index 6e42a4f..ba4a37d 100644 --- a/internal/plugin/reporter/github_pr.go +++ b/internal/plugin/reporter/github_pr.go @@ -50,7 +50,7 @@ func (s *GithubPullRequest) Write(changedLinesWithCoverage domain.SourceLineCove return errors.Wrap(bodyErr, "Failed creating payload for github") } - url := fmt.Sprintf("%v/api/v3/repos/%v/%v/issues/%v/comments", s.apiBaseURL, s.owner, s.repo, s.pr) + url := fmt.Sprintf("%v/repos/%v/%v/issues/%v/comments", strings.TrimRight(s.apiBaseURL, "/"), s.owner, s.repo, s.pr) req, newErr := s.httpClient.NewRequest( "POST", @@ -70,6 +70,10 @@ func (s *GithubPullRequest) Write(changedLinesWithCoverage domain.SourceLineCove return errors.Wrap(doErr, "Failed calling github") } + defer func() { + _ = resp.Body.Close() + }() + if resp.StatusCode != HTTPResponseCreated { return errors.Errorf("Failed calling github: bad status code: %v", resp.StatusCode) } @@ -85,47 +89,44 @@ func (s *GithubPullRequest) createCommentBody(changedLinesWithCoverage domain.So modules := collectModules(changedLinesWithCoverage) - summaryLines := []string{} - - if len(modules) > 0 { - summaryLines = append(summaryLines, fmt.Sprintf("*Modules: %v*\n\n", strings.Join(modules, ", "))) + bodyLines := []string{ + "## 📊 Code Coverage — Changed Lines\n", + "\n", + "> Coverage is measured **only for the lines this PR changes**, not the whole file or repo.\n", + "\n", } - var missedInstructions string - for _, r := range changedLinesWithCoverage { - if r.MissedInstructionCount > 0 { - missedInstructions += fmt.Sprintf("--- %v\n", lineDescription(r.SourceLine)) - missedInstructions += fmt.Sprintf("%v\n", r.LineValue) - } + if len(modules) > 0 { + bodyLines = append(bodyLines, fmt.Sprintf("*Modules: %v*\n\n", strings.Join(modules, ", "))) } - summaryLines = append(summaryLines, generateSummaryLines(changedLinesWithCoverage, func(linesWithDataCount int, linesWithoutDataCount int, covered int, missed int) []string { + bodyLines = append(bodyLines, generateSummaryLines(changedLinesWithCoverage, func(linesWithDataCount int, linesWithoutDataCount int, covered int, missed int) []string { totalLines := linesWithDataCount + linesWithoutDataCount totalInstructions := covered + missed - result := make([]string, 5) - - result[0] = fmt.Sprintf("Code Coverage Summary:\n\n") - result[1] = fmt.Sprintf("Lines Without Coverage Data -> %.f%% (%d)\n", toPercent(safeDiv(float32(linesWithoutDataCount), float32(totalLines), 0)), linesWithoutDataCount) - result[2] = fmt.Sprintf("Lines With Coverage Data -> %.f%% (%d)\n", toPercent(safeDiv(float32(linesWithDataCount), float32(totalLines), 1)), linesWithDataCount) - result[3] = fmt.Sprintf("Covered Instructions -> **%.f%%** (%d)\n", toPercent(safeDiv(float32(covered), float32(totalInstructions), 1)), covered) - result[4] = fmt.Sprintf("Missed Instructions -> %.f%% (%d)\n", toPercent(safeDiv(float32(missed), float32(totalInstructions), 0)), missed) - - return result + coveredPct := toPercent(safeDiv(float32(covered), float32(totalInstructions), 1)) + missedPct := toPercent(safeDiv(float32(missed), float32(totalInstructions), 0)) + withDataPct := toPercent(safeDiv(float32(linesWithDataCount), float32(totalLines), 1)) + withoutDataPct := toPercent(safeDiv(float32(linesWithoutDataCount), float32(totalLines), 0)) + + return []string{ + fmt.Sprintf("### %v Covered Instructions: %.f%% (%d)\n", coverageStatusEmoji(coveredPct), coveredPct, covered), + "\n", + "| Metric | Result | What it means |\n", + "| :-- | :-: | :-- |\n", + fmt.Sprintf("| 🟢 **Covered Instructions** | **%.f%%** (%d) | Changed code your tests executed. Higher is better. |\n", coveredPct, covered), + fmt.Sprintf("| 🔴 **Missed Instructions** | %.f%% (%d) | Changed code your tests never ran. Lower is better. |\n", missedPct, missed), + fmt.Sprintf("| 📈 Lines With Coverage Data | %.f%% (%d) | Changed lines the coverage tool could track. |\n", withDataPct, linesWithDataCount), + fmt.Sprintf("| ⚪ Lines Without Coverage Data | %.f%% (%d) | Changed lines with no data: comments, blanks, declarations. |\n", withoutDataPct, linesWithoutDataCount), + } })...) - var summary string - if missedInstructions == "" { - summary = strings.Join(summaryLines, "") - } else { - - summaryWithoutInstructions := strings.Join(summaryLines, "") - summary = summaryWithoutInstructions + "\n
Missed Instructions summary\n\n" + "```\n" + missedInstructions + "```" + - "\n
" - } + body := strings.Join(bodyLines, "") + body += missedInstructionsSection(changedLinesWithCoverage) + body += "\n🤖 Generated by pull-request-code-coverage — coverage for changed lines only.\n" data := map[string]string{ - "body": summary, + "body": body, } dataBytes, marshalErr := s.jsonClient.Marshal(data) @@ -137,6 +138,41 @@ func (s *GithubPullRequest) createCommentBody(changedLinesWithCoverage domain.So return bytes.NewBuffer(dataBytes), nil } +// missedInstructionsSection renders a collapsible block listing each changed +// line that was not executed by tests. Returns "" when nothing was missed. +func missedInstructionsSection(changedLinesWithCoverage domain.SourceLineCoverageReport) string { + var missedInstructions string + missedLineCount := 0 + + for _, r := range changedLinesWithCoverage { + if r.MissedInstructionCount > 0 { + missedLineCount++ + missedInstructions += fmt.Sprintf("--- %v\n", lineDescription(r.SourceLine)) + missedInstructions += fmt.Sprintf("%v\n", r.LineValue) + } + } + + if missedInstructions == "" { + return "" + } + + return fmt.Sprintf("\n
🔍 Missed instructions (%d)\n\n", missedLineCount) + + "```\n" + missedInstructions + "```" + "\n
\n" +} + +// coverageStatusEmoji maps a covered-instruction percentage to a traffic-light +// status icon, so the headline reads at a glance. +func coverageStatusEmoji(coveredPct float32) string { + switch { + case coveredPct >= 80: + return "🟢" + case coveredPct >= 50: + return "🟡" + default: + return "🔴" + } +} + func collectModules(changedLinesWithCoverage domain.SourceLineCoverageReport) []string { collector := map[string]bool{} diff --git a/internal/plugin/reporter/github_pr_test.go b/internal/plugin/reporter/github_pr_test.go index f1777c1..be239c1 100644 --- a/internal/plugin/reporter/github_pr_test.go +++ b/internal/plugin/reporter/github_pr_test.go @@ -1,8 +1,10 @@ package reporter import ( + "io" "net/http" "net/http/httptest" + "strings" "testing" "github.com/pkg/errors" @@ -66,7 +68,7 @@ func TestGithubPullRequest_Write_FailedDo_BadStatus(t *testing.T) { request := httptest.NewRequest("GET", "http://anywhere", nil) mockClient.On("NewRequest", mock.Anything, mock.Anything, mock.Anything).Return(request, nil) - mockClient.On("Do", request).Return(&http.Response{StatusCode: 400}, nil) + mockClient.On("Do", request).Return(&http.Response{StatusCode: 400, Body: io.NopCloser(strings.NewReader(""))}, nil) writer := &GithubPullRequest{ apiBaseURL: "anything", @@ -85,6 +87,50 @@ func TestGithubPullRequest_Write_FailedDo_BadStatus(t *testing.T) { assert.EqualError(t, e, "Failed calling github: bad status code: 400") } +func TestGithubPullRequest_Write_BuildsPublicGithubURL(t *testing.T) { + + mockClient := &pluginhttp.MockClient{} + request := httptest.NewRequest("POST", "http://anywhere", nil) + + mockClient.On("NewRequest", "POST", "https://api.github.com/repos/some_owner/some_repo/issues/42/comments", mock.Anything).Return(request, nil) + mockClient.On("Do", request).Return(&http.Response{StatusCode: 201, Body: io.NopCloser(strings.NewReader(""))}, nil) + + writer := NewGithubPullRequest("KEY", "https://api.github.com", "42", "some_owner", "some_repo", mockClient, &pluginjson.DefaultClient{}) + + e := writer.Write(domain.SourceLineCoverageReport{ + domain.SourceLineCoverage{ + CoverageData: domain.CoverageData{ + CoveredInstructionCount: 1, + }, + }, + }) + + assert.NoError(t, e) + mockClient.AssertExpectations(t) +} + +func TestGithubPullRequest_Write_TrimsTrailingSlashFromEnterpriseURL(t *testing.T) { + + mockClient := &pluginhttp.MockClient{} + request := httptest.NewRequest("POST", "http://anywhere", nil) + + mockClient.On("NewRequest", "POST", "https://git.target.com/api/v3/repos/some_owner/some_repo/issues/42/comments", mock.Anything).Return(request, nil) + mockClient.On("Do", request).Return(&http.Response{StatusCode: 201, Body: io.NopCloser(strings.NewReader(""))}, nil) + + writer := NewGithubPullRequest("KEY", "https://git.target.com/api/v3/", "42", "some_owner", "some_repo", mockClient, &pluginjson.DefaultClient{}) + + e := writer.Write(domain.SourceLineCoverageReport{ + domain.SourceLineCoverage{ + CoverageData: domain.CoverageData{ + CoveredInstructionCount: 1, + }, + }, + }) + + assert.NoError(t, e) + mockClient.AssertExpectations(t) +} + func TestGithubPullRequest_Write_FailedJsonMarshal(t *testing.T) { mockClient := &pluginjson.MockClient{} diff --git a/internal/plugin/reporter/simple.go b/internal/plugin/reporter/simple.go index 7984175..55f6e96 100644 --- a/internal/plugin/reporter/simple.go +++ b/internal/plugin/reporter/simple.go @@ -38,7 +38,7 @@ func (s *Simple) Write(changedLinesWithCoverage domain.SourceLineCoverageReport) result := make([]string, 5) - result[0] = fmt.Sprintf("Code Coverage Summary:\n") + result[0] = "Code Coverage Summary:\n" result[1] = fmt.Sprintf("Lines Without Coverage Data -> %.f%% (%d)\n", toPercent(safeDiv(float32(linesWithoutDataCount), float32(totalLines), 0)), linesWithoutDataCount) result[2] = fmt.Sprintf("Lines With Coverage Data -> %.f%% (%d)\n", toPercent(safeDiv(float32(linesWithDataCount), float32(totalLines), 1)), linesWithDataCount) result[3] = fmt.Sprintf("Covered Instructions -> %.f%% (%d)\n", toPercent(safeDiv(float32(covered), float32(totalInstructions), 1)), covered) diff --git a/internal/plugin/runner.go b/internal/plugin/runner.go index 032a99b..2d3c5c9 100644 --- a/internal/plugin/runner.go +++ b/internal/plugin/runner.go @@ -18,6 +18,11 @@ import ( "github.com/target/pull-request-code-coverage/internal/plugin/sourcelines/unifieddiff" ) +// defaultGithubAPIBaseURL is the public GitHub REST API root. GitHub Enterprise +// users should set PARAMETER_GH_API_BASE_URL to their API root (e.g. +// https://git.example.com/api/v3). +const defaultGithubAPIBaseURL = "https://api.github.com" + type DefaultRunner struct{} func NewRunner() *DefaultRunner { @@ -28,6 +33,8 @@ func NewRunner() *DefaultRunner { // nolint: gocyclo func (*DefaultRunner) Run(propertyGetter func(string) (string, bool), changedSourceLinesSource io.Reader, reportDefaultOut io.Writer) error { + logrus.Info("starting pull-request-code-coverage run") + rawSourceDirs, found := propertyGetter("PARAMETER_SOURCE_DIRS") if !found { return errors.New("Missing property PARAMETER_SOURCE_DIRS") @@ -66,8 +73,9 @@ func (*DefaultRunner) Run(propertyGetter func(string) (string, bool), changedSou } ghAPIBaseURL, ghAPIBaseURLFound := propertyGetter("PARAMETER_GH_API_BASE_URL") - if !ghAPIBaseURLFound { - logrus.Info("PARAMETER_GH_API_BASE_URL was missing, will not send report to PR comments") + if !ghAPIBaseURLFound || ghAPIBaseURL == "" { + ghAPIBaseURL = defaultGithubAPIBaseURL + logrus.Info(fmt.Sprintf("PARAMETER_GH_API_BASE_URL was missing, defaulting to %v", ghAPIBaseURL)) } repoPR, repoPRFound := propertyGetter("BUILD_PULL_REQUEST_NUMBER") @@ -112,7 +120,7 @@ func (*DefaultRunner) Run(propertyGetter func(string) (string, bool), changedSou reporters := []reporter.Reporter{reporter.NewSimple(reportDefaultOut)} - if ghAPIKeyFound && ghAPIBaseURLFound && repoPRFound && repoOwnerFound && repoNameFound { + if ghAPIKeyFound && repoPRFound && repoOwnerFound && repoNameFound { reporters = append(reporters, reporter.NewGithubPullRequest(ghAPIKey, ghAPIBaseURL, repoPR, repoOwner, repoName, &pluginhttp.DefaultClient{}, &pluginjson.DefaultClient{})) } logrus.Info("enabled reporters are ") diff --git a/internal/plugin/runner_test.go b/internal/plugin/runner_test.go index 7529ed6..e1f64f0 100644 --- a/internal/plugin/runner_test.go +++ b/internal/plugin/runner_test.go @@ -107,15 +107,21 @@ Covered Instructions -> 97% (177) Missed Instructions -> 3% (5) `, buf.String()) - requestAsserter.AssertRequestWasMade(t, "/api/v3/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ - "body": `Code Coverage Summary: + requestAsserter.AssertRequestWasMade(t, "/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ + "body": `## 📊 Code Coverage — Changed Lines -Lines Without Coverage Data -> 92% (2216) -Lines With Coverage Data -> 8% (182) -Covered Instructions -> **97%** (177) -Missed Instructions -> 3% (5) +> Coverage is measured **only for the lines this PR changes**, not the whole file or repo. -
Missed Instructions summary +### 🟢 Covered Instructions: 97% (177) + +| Metric | Result | What it means | +| :-- | :-: | :-- | +| 🟢 **Covered Instructions** | **97%** (177) | Changed code your tests executed. Higher is better. | +| 🔴 **Missed Instructions** | 3% (5) | Changed code your tests never ran. Lower is better. | +| 📈 Lines With Coverage Data | 8% (182) | Changed lines the coverage tool could track. | +| ⚪ Lines Without Coverage Data | 92% (2216) | Changed lines with no data: comments, blanks, declarations. | + +
🔍 Missed instructions (5) ` + "```" + ` --- internal/plugin/runner.go:72 @@ -129,7 +135,11 @@ func GetCoverageReportLoader(coverageType string, sourceDir string) coverage.Loa --- main.go:17 os.Exit(1) ` + - "```\n
", + "```" + ` +
+ +🤖 Generated by pull-request-code-coverage — coverage for changed lines only. +`, }) propGetter.AssertExpectations(t) @@ -178,15 +188,21 @@ Covered Instructions -> 97% (177) Missed Instructions -> 3% (5) `, buf.String()) - requestAsserter.AssertRequestWasMade(t, "/api/v3/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ - "body": `Code Coverage Summary: + requestAsserter.AssertRequestWasMade(t, "/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ + "body": `## 📊 Code Coverage — Changed Lines -Lines Without Coverage Data -> 92% (2216) -Lines With Coverage Data -> 8% (182) -Covered Instructions -> **97%** (177) -Missed Instructions -> 3% (5) +> Coverage is measured **only for the lines this PR changes**, not the whole file or repo. + +### 🟢 Covered Instructions: 97% (177) + +| Metric | Result | What it means | +| :-- | :-: | :-- | +| 🟢 **Covered Instructions** | **97%** (177) | Changed code your tests executed. Higher is better. | +| 🔴 **Missed Instructions** | 3% (5) | Changed code your tests never ran. Lower is better. | +| 📈 Lines With Coverage Data | 8% (182) | Changed lines the coverage tool could track. | +| ⚪ Lines Without Coverage Data | 92% (2216) | Changed lines with no data: comments, blanks, declarations. | -
Missed Instructions summary +
🔍 Missed instructions (5) ` + "```" + ` --- internal/plugin/runner.go:72 @@ -200,7 +216,11 @@ func GetCoverageReportLoader(coverageType string, sourceDir string) coverage.Loa --- main.go:17 os.Exit(1) ` + - "```\n
", + "```" + ` +
+ +🤖 Generated by pull-request-code-coverage — coverage for changed lines only. +`, }) propGetter.AssertExpectations(t) @@ -239,23 +259,33 @@ Covered Instructions -> 73% (8) Missed Instructions -> 27% (3) `, buf.String()) - requestAsserter.AssertRequestWasMade(t, "/api/v3/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ - "body": `*Modules: category-search* + requestAsserter.AssertRequestWasMade(t, "/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ + "body": `## 📊 Code Coverage — Changed Lines -Code Coverage Summary: +> Coverage is measured **only for the lines this PR changes**, not the whole file or repo. -Lines Without Coverage Data -> 78% (7) -Lines With Coverage Data -> 22% (2) -Covered Instructions -> **73%** (8) -Missed Instructions -> 27% (3) +*Modules: category-search* -
Missed Instructions summary +### 🟡 Covered Instructions: 73% (8) + +| Metric | Result | What it means | +| :-- | :-: | :-- | +| 🟢 **Covered Instructions** | **73%** (8) | Changed code your tests executed. Higher is better. | +| 🔴 **Missed Instructions** | 27% (3) | Changed code your tests never ran. Lower is better. | +| 📈 Lines With Coverage Data | 22% (2) | Changed lines the coverage tool could track. | +| ⚪ Lines Without Coverage Data | 78% (7) | Changed lines with no data: comments, blanks, declarations. | + +
🔍 Missed instructions (1) ` + "```" + ` --- category-search/src/main/java/com/tgt/CategorySearchApplication.java:52 System.out.print("Something"); ` + - "```\n
", + "```" + ` +
+ +🤖 Generated by pull-request-code-coverage — coverage for changed lines only. +`, }) propGetter.AssertExpectations(t) @@ -295,23 +325,33 @@ Covered Instructions -> 73% (8) Missed Instructions -> 27% (3) `, buf.String()) - requestAsserter.AssertRequestWasMade(t, "/api/v3/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ - "body": `*Modules: category-search* + requestAsserter.AssertRequestWasMade(t, "/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ + "body": `## 📊 Code Coverage — Changed Lines -Code Coverage Summary: +> Coverage is measured **only for the lines this PR changes**, not the whole file or repo. -Lines Without Coverage Data -> 78% (7) -Lines With Coverage Data -> 22% (2) -Covered Instructions -> **73%** (8) -Missed Instructions -> 27% (3) +*Modules: category-search* + +### 🟡 Covered Instructions: 73% (8) -
Missed Instructions summary +| Metric | Result | What it means | +| :-- | :-: | :-- | +| 🟢 **Covered Instructions** | **73%** (8) | Changed code your tests executed. Higher is better. | +| 🔴 **Missed Instructions** | 27% (3) | Changed code your tests never ran. Lower is better. | +| 📈 Lines With Coverage Data | 22% (2) | Changed lines the coverage tool could track. | +| ⚪ Lines Without Coverage Data | 78% (7) | Changed lines with no data: comments, blanks, declarations. | + +
🔍 Missed instructions (1) ` + "```" + ` --- category-search/src/main/java/com/tgt/CategorySearchApplication.java:52 System.out.print("Something"); ` + - "```\n
", + "```" + ` +
+ +🤖 Generated by pull-request-code-coverage — coverage for changed lines only. +`, }) propGetter.AssertExpectations(t) @@ -353,24 +393,34 @@ Covered Instructions -> 88% (42) Missed Instructions -> 12% (6) `, buf.String()) - requestAsserter.AssertRequestWasMade(t, "/api/v3/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ - "body": `*Modules: category-search* + requestAsserter.AssertRequestWasMade(t, "/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ + "body": `## 📊 Code Coverage — Changed Lines -Code Coverage Summary: +> Coverage is measured **only for the lines this PR changes**, not the whole file or repo. -Lines Without Coverage Data -> 47% (7) -Lines With Coverage Data -> 53% (8) -Covered Instructions -> **88%** (42) -Missed Instructions -> 12% (6) +*Modules: category-search* + +### 🟢 Covered Instructions: 88% (42) -
Missed Instructions summary +| Metric | Result | What it means | +| :-- | :-: | :-- | +| 🟢 **Covered Instructions** | **88%** (42) | Changed code your tests executed. Higher is better. | +| 🔴 **Missed Instructions** | 12% (6) | Changed code your tests never ran. Lower is better. | +| 📈 Lines With Coverage Data | 53% (8) | Changed lines the coverage tool could track. | +| ⚪ Lines Without Coverage Data | 47% (7) | Changed lines with no data: comments, blanks, declarations. | + +
🔍 Missed instructions (2) ` + "```" + ` --- category-search/src/main/java/com/tgt/CategorySearchApplication.java:52 System.out.print("Something"); --- category-search/src/main/kotlin/com/tgt/SomeOtherClass.kt:12 System.out.print("Something2"); -` + "```\n
", +` + "```" + ` +
+ +🤖 Generated by pull-request-code-coverage — coverage for changed lines only. +`, }) propGetter.AssertExpectations(t) @@ -413,17 +463,23 @@ Covered Instructions -> 88% (42) Missed Instructions -> 12% (6) `, buf.String()) - requestAsserter.AssertRequestWasMade(t, "/api/v3/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ - "body": `*Modules: category-search* + requestAsserter.AssertRequestWasMade(t, "/repos/some_org/some_repo/issues/123/comments", "SOME_API_KEY", map[string]interface{}{ + "body": `## 📊 Code Coverage — Changed Lines -Code Coverage Summary: +> Coverage is measured **only for the lines this PR changes**, not the whole file or repo. -Lines Without Coverage Data -> 47% (7) -Lines With Coverage Data -> 53% (8) -Covered Instructions -> **88%** (42) -Missed Instructions -> 12% (6) +*Modules: category-search* + +### 🟢 Covered Instructions: 88% (42) -
Missed Instructions summary +| Metric | Result | What it means | +| :-- | :-: | :-- | +| 🟢 **Covered Instructions** | **88%** (42) | Changed code your tests executed. Higher is better. | +| 🔴 **Missed Instructions** | 12% (6) | Changed code your tests never ran. Lower is better. | +| 📈 Lines With Coverage Data | 53% (8) | Changed lines the coverage tool could track. | +| ⚪ Lines Without Coverage Data | 47% (7) | Changed lines with no data: comments, blanks, declarations. | + +
🔍 Missed instructions (2) ` + "```" + ` --- category-search/src/main/java/com/tgt/CategorySearchApplication.java:52 @@ -431,7 +487,11 @@ Missed Instructions -> 12% (6) --- category-search/src/main/kotlin/com/tgt/SomeOtherClass.kt:12 System.out.print("Something2"); ` + - "```\n
", + "```" + ` +
+ +🤖 Generated by pull-request-code-coverage — coverage for changed lines only. +`, }) propGetter.AssertExpectations(t) diff --git a/internal/test/mocks/mock_gh_api.go b/internal/test/mocks/mock_gh_api.go index 0b87bc4..ae99bf2 100644 --- a/internal/test/mocks/mock_gh_api.go +++ b/internal/test/mocks/mock_gh_api.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "io" - "io/ioutil" "net/http" "net/http/httptest" "testing" @@ -84,7 +83,7 @@ func mustJSONUnmarshall(bytes []byte, result interface{}) { } func mustReadAll(r io.Reader) []byte { - result, err := ioutil.ReadAll(r) + result, err := io.ReadAll(r) if err != nil { panic(fmt.Sprintf("Unexpected error: %v", err))