diff --git a/.github/workflows/daily-cli-tests.yml b/.github/workflows/daily-cli-tests.yml index 223f7ac5d..6294fd701 100644 --- a/.github/workflows/daily-cli-tests.yml +++ b/.github/workflows/daily-cli-tests.yml @@ -54,8 +54,8 @@ jobs: kosli_querying_api_token: ${{ secrets.KOSLI_API_TOKEN_PROD }} sonarqube_token: ${{ secrets.KOSLI_SONARQUBE_TOKEN }} - aws-contract-tests: - name: AWS Contract Tests + contract-tests: + name: Contract Tests runs-on: ubuntu-latest permissions: id-token: write @@ -81,8 +81,10 @@ jobs: role-duration-seconds: 2400 role-session-name: ${{ github.event.repository.name }} - - name: Run AWS contract tests - run: make test_smoke_aws + - name: Run contract tests + run: make test_contract + env: + KOSLI_GITHUB_TOKEN: ${{ secrets.KOSLI_GITHUB_TOKEN }} slack-notification-on-failure: runs-on: ubuntu-24.04 @@ -93,7 +95,7 @@ jobs: [ set-trail-name, test, - aws-contract-tests, + contract-tests, ] if: ${{ always() && contains(join(needs.*.result, ','), 'failure') && github.ref == 'refs/heads/main' }} steps: diff --git a/Makefile b/Makefile index 2d4792f37..078c1ab44 100644 --- a/Makefile +++ b/Makefile @@ -149,6 +149,13 @@ test_smoke_aws: ensure_gotestsum ## Run AWS contract and smoke tests against rea @echo "Requires AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY to be set" @$(GOTESTSUM) -- -v -p=1 -run "LambdaContract_RealAWS|AWSTestSuite/TestGetLambdaPackageData|AWSTestSuite/TestGetEcsTasksData|AWSTestSuite/TestGetS3Data" ./internal/aws/ +test_smoke_github: ensure_gotestsum ## Run GitHub contract tests against real GitHub API (requires KOSLI_GITHUB_TOKEN) + @echo "Running GitHub contract tests against real GitHub API..." + @echo "Requires KOSLI_GITHUB_TOKEN to be set" + @$(GOTESTSUM) -- -v -p=1 -run "GitHubContract_RealGitHub" ./internal/github/ + +test_contract: test_smoke_aws test_smoke_github ## Run all contract tests against real external services + test_docs: deps vet ensure_network test_setup ## Test docs ./bin/test_docs_cmds.sh docs.kosli.com/content/use_cases/simulating_a_devops_system/_index.md diff --git a/TODO.md b/TODO.md index 2c3ed9d3f..0229b8b87 100644 --- a/TODO.md +++ b/TODO.md @@ -55,6 +55,30 @@ - [x] Slice 3: Show params in `--show-input` output - [x] Slice 4: Update help text and examples +## Fakes & contract tests for GitHub API integration + +### Slice 1: FakeGitHubClient + contract tests (`internal/github`) ← active + +- [x] `TestGitHubContract_Fake`: V2 returns PRs for commit with PRs +- [x] `TestGitHubContract_Fake`: V2 returns empty for commit with no PRs +- [x] `TestGitHubContract_Fake`: V2 returns error when Err is injected +- [x] `TestGitHubContract_Fake`: V1 returns PRs for commit with PRs +- [x] `TestGitHubContract_Fake`: V1 returns empty for commit with no PRs +- [x] `TestGitHubContract_Fake`: V1 returns error when Err is injected +- [x] `TestGitHubContract_RealGitHub`: same contract, env-gated on `KOSLI_GITHUB_TOKEN` + +### Slice 2: Thread fake through command layer ← active + +- [x] Add `ProviderAndLabel() (string, string)` to `types.PRRetriever` interface +- [x] Implement on `GithubConfig` → `("github", "pull request")` +- [x] Implement on `GitlabConfig` → `("gitlab", "merge request")` +- [x] Implement on `AzureConfig` → `("azure", "pull request")` +- [x] Implement on bitbucket `Config` → `("bitbucket", "pull request")` +- [x] Implement on `FakeGitHubClient` → `("github", "pull request")` +- [x] Replace reflection in `getGitProviderAndLabel` with `retriever.ProviderAndLabel()` +- [x] Inject fake in `assertPRGithub_test.go` +- [x] Inject fake in `attestPRGithub_test.go` + ## Fakes & contract tests for cloud provider integrations (#758) ### Lambda (done — this PR) diff --git a/cmd/kosli/assertPRGithub.go b/cmd/kosli/assertPRGithub.go index a74243128..05241c89f 100644 --- a/cmd/kosli/assertPRGithub.go +++ b/cmd/kosli/assertPRGithub.go @@ -5,18 +5,19 @@ import ( "io" ghUtils "github.com/kosli-dev/cli/internal/github" + "github.com/kosli-dev/cli/internal/types" "github.com/spf13/cobra" ) type assertPullRequestGithubOptions struct { - githubConfig *ghUtils.GithubConfig - commit string + retriever types.PRRetriever + commit string } const assertPRGithubShortDesc = `Assert a Github pull request for a git commit exists. ` const assertPRGithubLongDesc = assertPRGithubShortDesc + ` -The command exits with non-zero exit code +The command exits with non-zero exit code if no pull requests were found for the commit.` const assertPRGithubExample = ` @@ -38,7 +39,7 @@ func newAssertPullRequestGithubCmd(out io.Writer) *cobra.Command { Example: assertPRGithubExample, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - o.githubConfig = ghUtils.NewGithubConfig(githubFlagsValues.Token, githubFlagsValues.BaseURL, + o.retriever = ghUtils.NewGithubRetrieverFunc(githubFlagsValues.Token, githubFlagsValues.BaseURL, githubFlagsValues.Org, githubFlagsValues.Repository) return o.run(args) }, @@ -58,7 +59,7 @@ func newAssertPullRequestGithubCmd(out io.Writer) *cobra.Command { } func (o *assertPullRequestGithubOptions) run(args []string) error { - pullRequestsEvidence, err := o.githubConfig.PREvidenceForCommitV2(o.commit) + pullRequestsEvidence, err := o.retriever.PREvidenceForCommitV2(o.commit) if err != nil { return err } diff --git a/cmd/kosli/assertPRGithub_test.go b/cmd/kosli/assertPRGithub_test.go index ce2919c24..c846fa481 100644 --- a/cmd/kosli/assertPRGithub_test.go +++ b/cmd/kosli/assertPRGithub_test.go @@ -4,27 +4,40 @@ import ( "fmt" "testing" - "github.com/kosli-dev/cli/internal/testHelpers" + ghUtils "github.com/kosli-dev/cli/internal/github" + "github.com/kosli-dev/cli/internal/types" "github.com/stretchr/testify/suite" ) -// Define the suite, and absorb the built-in basic suite -// functionality from testify - including a T() method which -// returns the current testing context type AssertPRGithubCommandTestSuite struct { suite.Suite defaultKosliArguments string + commitWithPR string + commitWithNoPR string } func (suite *AssertPRGithubCommandTestSuite) SetupTest() { - testHelpers.SkipIfEnvVarUnset(suite.T(), []string{"KOSLI_GITHUB_TOKEN"}) + suite.commitWithPR = "480e5a00379a52b8e184d6815080242a878ca295" + suite.commitWithNoPR = "7d1db1c8b7e71ee0ce369f1b722cc8844d3a7af6" + + ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string) types.PRRetriever { + return &ghUtils.FakeGitHubClient{ + PRsByCommit: map[string][]*types.PREvidence{ + suite.commitWithPR: {{URL: "https://github.com/kosli-dev/cli/pull/1", State: "MERGED"}}, + }, + } + } global = &GlobalOpts{ ApiToken: "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ImNkNzg4OTg5In0.e8i_lA_QrEhFncb05Xw6E_tkCHU9QfcY4OLTVUCHffY", Org: "docs-cmd-test-user", Host: "http://localhost:8001", } - suite.defaultKosliArguments = fmt.Sprintf(" --host %s --org %s --api-token %s", global.Host, global.Org, global.ApiToken) + suite.defaultKosliArguments = fmt.Sprintf(" --github-token fake --host %s --org %s --api-token %s", global.Host, global.Org, global.ApiToken) +} + +func (suite *AssertPRGithubCommandTestSuite) TearDownTest() { + ghUtils.ResetGithubRetrieverFunc() } func (suite *AssertPRGithubCommandTestSuite) TestAssertPRGithubCmd() { @@ -32,30 +45,28 @@ func (suite *AssertPRGithubCommandTestSuite) TestAssertPRGithubCmd() { { name: "assert Github PR evidence passes when commit has a PR in github", cmd: `assert pullrequest github --github-org kosli-dev --repository cli - --commit ` + testHelpers.GithubCommitWithPR() + suite.defaultKosliArguments, - golden: fmt.Sprintf("found [1] pull request(s) in Github for commit: %s\n", testHelpers.GithubCommitWithPR()), + --commit ` + suite.commitWithPR + suite.defaultKosliArguments, + golden: fmt.Sprintf("found [1] pull request(s) in Github for commit: %s\n", suite.commitWithPR), }, { wantError: true, name: "assert Github PR evidence fails when commit has no PRs in github", - cmd: `assert pullrequest github --github-org kosli-dev --repository cli - --commit 19aab7f063147614451c88969602a10afbabb43d` + suite.defaultKosliArguments, - golden: "Error: assert failed: found no pull request(s) in Github for commit: 19aab7f063147614451c88969602a10afbabb43d\n", + cmd: `assert pullrequest github --github-org kosli-dev --repository cli + --commit ` + suite.commitWithNoPR + suite.defaultKosliArguments, + golden: fmt.Sprintf("Error: assert failed: found no pull request(s) in Github for commit: %s\n", suite.commitWithNoPR), }, { wantError: true, name: "assert Github PR evidence fails when commit does not exist", - cmd: `assert pullrequest github --github-org kosli-dev --repository cli - --commit 19aab7f063147614451c88969602a10afba123ab` + suite.defaultKosliArguments, - golden: "Error: assert failed: found no pull request(s) in Github for commit: 19aab7f063147614451c88969602a10afba123ab\n", + cmd: `assert pullrequest github --github-org kosli-dev --repository cli + --commit 0000000000000000000000000000000000000000` + suite.defaultKosliArguments, + golden: "Error: assert failed: found no pull request(s) in Github for commit: 0000000000000000000000000000000000000000\n", }, } runTestCmd(suite.T(), tests) } -// In order for 'go test' to run this suite, we need to create -// a normal test function and pass our suite to suite.Run func TestAssertPRGithubCommandTestSuite(t *testing.T) { suite.Run(t, new(AssertPRGithubCommandTestSuite)) } diff --git a/cmd/kosli/attestPRGithub.go b/cmd/kosli/attestPRGithub.go index 1c192fddf..a4c345c70 100644 --- a/cmd/kosli/attestPRGithub.go +++ b/cmd/kosli/attestPRGithub.go @@ -142,7 +142,7 @@ func newAttestGithubPRCmd(out io.Writer) *cobra.Command { }, RunE: func(cmd *cobra.Command, args []string) error { o.repoURLExplicit = cmd.Flags().Changed("repo-url") - o.retriever = ghUtils.NewGithubConfig(githubFlagsValues.Token, githubFlagsValues.BaseURL, + o.retriever = ghUtils.NewGithubRetrieverFunc(githubFlagsValues.Token, githubFlagsValues.BaseURL, githubFlagsValues.Org, o.repoName) return o.run(args) }, diff --git a/cmd/kosli/attestPRGithub_test.go b/cmd/kosli/attestPRGithub_test.go index 0d33646dd..9b9ea95c1 100644 --- a/cmd/kosli/attestPRGithub_test.go +++ b/cmd/kosli/attestPRGithub_test.go @@ -4,13 +4,11 @@ import ( "fmt" "testing" - "github.com/kosli-dev/cli/internal/testHelpers" + ghUtils "github.com/kosli-dev/cli/internal/github" + "github.com/kosli-dev/cli/internal/types" "github.com/stretchr/testify/suite" ) -// Define the suite, and absorb the built-in basic suite -// functionality from testify - including a T() method which -// returns the current testing context type AttestGithubPRCommandTestSuite struct { flowName string trailName string @@ -22,24 +20,35 @@ type AttestGithubPRCommandTestSuite struct { } func (suite *AttestGithubPRCommandTestSuite) SetupTest() { - testHelpers.SkipIfEnvVarUnset(suite.T(), []string{"KOSLI_GITHUB_TOKEN"}) - suite.flowName = "attest-github-pr" suite.trailName = "test-123" suite.artifactFingerprint = "7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9" - suite.commitWithPR = "a72d2b5cfae42cb95700b3645de0c8ba3129a2ae" - suite.commitWithNoPR = "13c900483c17b6ca5e0b26984ed74a6120838cad" + suite.commitWithPR = "480e5a00379a52b8e184d6815080242a878ca295" + suite.commitWithNoPR = "7d1db1c8b7e71ee0ce369f1b722cc8844d3a7af6" + + ghUtils.NewGithubRetrieverFunc = func(token, baseURL, org, repository string) types.PRRetriever { + return &ghUtils.FakeGitHubClient{ + PRsByCommit: map[string][]*types.PREvidence{ + suite.commitWithPR: {{URL: "https://github.com/kosli-dev/cli/pull/1", State: "MERGED"}}, + }, + } + } + global = &GlobalOpts{ ApiToken: "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpZCI6ImNkNzg4OTg5In0.e8i_lA_QrEhFncb05Xw6E_tkCHU9QfcY4OLTVUCHffY", Org: "docs-cmd-test-user", Host: "http://localhost:8001", } - suite.defaultKosliArguments = fmt.Sprintf(" --flow %s --trail %s --repo-root ../.. --host %s --org %s --api-token %s", suite.flowName, suite.trailName, global.Host, global.Org, global.ApiToken) + suite.defaultKosliArguments = fmt.Sprintf(" --github-token fake --flow %s --trail %s --repo-root ../.. --host %s --org %s --api-token %s", suite.flowName, suite.trailName, global.Host, global.Org, global.ApiToken) CreateFlowWithTemplate(suite.flowName, "testdata/valid_template.yml", suite.T()) BeginTrail(suite.trailName, suite.flowName, "", suite.T()) CreateArtifactOnTrail(suite.flowName, suite.trailName, "cli", suite.artifactFingerprint, "file1", suite.T()) } +func (suite *AttestGithubPRCommandTestSuite) TearDownTest() { + ghUtils.ResetGithubRetrieverFunc() +} + func (suite *AttestGithubPRCommandTestSuite) TestAttestGithubPRCmd() { tests := []cmdTestCase{ { @@ -69,56 +78,56 @@ func (suite *AttestGithubPRCommandTestSuite) TestAttestGithubPRCmd() { { wantError: true, name: "05 fails when --commit is provided as empty string", - cmd: fmt.Sprintf(`attest pullrequest github --commit "" --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo + cmd: fmt.Sprintf(`attest pullrequest github --commit "" --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --github-org kosli-dev --repository cli %s`, suite.defaultKosliArguments), golden: "Error: flag '--commit' is required, but empty string was provided\n", }, { wantError: true, name: "06 attesting against an artifact that does not exist fails", - cmd: fmt.Sprintf(`attest pullrequest github --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo + cmd: fmt.Sprintf(`attest pullrequest github --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\nError: Artifact with fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 does not exist in trail \"test-123\" of flow \"attest-github-pr\" belonging to organization \"docs-cmd-test-user\"\n", }, { name: "07 can attest github pr against an artifact using artifact name and --artifact-type", - cmd: fmt.Sprintf(`attest pullrequest github testdata/file1 --artifact-type file --name foo + cmd: fmt.Sprintf(`attest pullrequest github testdata/file1 --artifact-type file --name foo --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'foo' is reported to trail: test-123\n", }, { name: "08 can attest github pr against an artifact using artifact name and --artifact-type when --name does not exist in the trail template", - cmd: fmt.Sprintf(`attest pullrequest github testdata/file1 --artifact-type file --name bar + cmd: fmt.Sprintf(`attest pullrequest github testdata/file1 --artifact-type file --name bar --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\n", }, { name: "09 can attest github pr against an artifact using --fingerprint", - cmd: fmt.Sprintf(`attest pullrequest github --fingerprint 7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo + cmd: fmt.Sprintf(`attest pullrequest github --fingerprint 7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'foo' is reported to trail: test-123\n", }, { name: "10 can attest github pr against a trail", - cmd: fmt.Sprintf(`attest pullrequest github --name bar + cmd: fmt.Sprintf(`attest pullrequest github --name bar --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\n", }, { name: "11 can attest github pr against a trail when name is not found in the trail template", - cmd: fmt.Sprintf(`attest pullrequest github --name additional + cmd: fmt.Sprintf(`attest pullrequest github --name additional --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'additional' is reported to trail: test-123\n", }, { name: "12 can attest github pr against an artifact it is created using dot syntax in --name", - cmd: fmt.Sprintf(`attest pullrequest github --name cli.foo + cmd: fmt.Sprintf(`attest pullrequest github --name cli.foo --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'foo' is reported to trail: test-123\n", }, { - name: "13 can attest github pr with external-url and external-fingerprint against a trail ", - cmd: fmt.Sprintf(`attest pullrequest github --name bar + name: "13 can attest github pr with external-url and external-fingerprint against a trail", + cmd: fmt.Sprintf(`attest pullrequest github --name bar --external-url file=https://example.com/file --external-fingerprint file=7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --github-org kosli-dev --repository cli --commit %s %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\n", @@ -126,20 +135,20 @@ func (suite *AttestGithubPRCommandTestSuite) TestAttestGithubPRCmd() { { wantError: true, name: "14 assert fails with non-zero exit code when commit has no PRs", - cmd: fmt.Sprintf(`attest pullrequest github --name bar + cmd: fmt.Sprintf(`attest pullrequest github --name bar --github-org kosli-dev --repository cli --commit %s --assert %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\nError: assert failed: no pull request found for the given commit: .*\n", }, { name: "15 assert works and has zero exit code when commit has PR(s)", - cmd: fmt.Sprintf(`attest pullrequest github --name bar + cmd: fmt.Sprintf(`attest pullrequest github --name bar --github-org kosli-dev --repository cli --commit %s --assert %s`, suite.commitWithPR, suite.defaultKosliArguments), goldenRegex: "found 1 pull request\\(s\\) for commit: .*\ngithub pull request attestation 'bar' is reported to trail: test-123\n", }, { wantError: true, name: "16 if there is a server error, this is output even when assert fails", - cmd: fmt.Sprintf(`attest pullrequest github --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo + cmd: fmt.Sprintf(`attest pullrequest github --fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 --name foo --github-org kosli-dev --repository cli --commit %s --assert %s`, suite.commitWithNoPR, suite.defaultKosliArguments), goldenRegex: "found 0 pull request\\(s\\) for commit: .*\nError: Artifact with fingerprint 1234e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9 does not exist in trail \"test-123\" of flow \"attest-github-pr\" belonging to organization \"docs-cmd-test-user\"\nError: assert failed: no pull request found for the given commit: .*\n", }, @@ -152,19 +161,19 @@ func (suite *AttestGithubPRCommandTestSuite) TestAttestGithubPRCmd() { { wantError: true, name: "18 fails when --repo-url is not a valid URL", - cmd: fmt.Sprintf("attest pullrequest github --name foo --commit %s --github-token fake --github-org myorg --repository myrepo --repo-url not-a-url %s", suite.commitWithPR, suite.defaultKosliArguments), + cmd: fmt.Sprintf("attest pullrequest github --name foo --commit %s --github-org myorg --repository myrepo --repo-url not-a-url %s", suite.commitWithPR, suite.defaultKosliArguments), golden: "Error: --repo-url 'not-a-url' is not a valid URL\n", }, { wantError: true, name: "19 fails when --repo-provider is not an allowed value", - cmd: fmt.Sprintf("attest pullrequest github --name foo --commit %s --github-token fake --github-org myorg --repository myrepo --repo-provider jenkins %s", suite.commitWithPR, suite.defaultKosliArguments), + cmd: fmt.Sprintf("attest pullrequest github --name foo --commit %s --github-org myorg --repository myrepo --repo-provider jenkins %s", suite.commitWithPR, suite.defaultKosliArguments), golden: "Error: --repo-provider 'jenkins' is not allowed. Must be one of: github, gitlab, bitbucket, azure-devops\n", }, { wantError: true, name: "20 fails when --name has invalid dot format", - cmd: fmt.Sprintf("attest pullrequest github --name .foo --github-org myorg --commit %s --github-token fake --repository myrepo %s", suite.commitWithPR, suite.defaultKosliArguments), + cmd: fmt.Sprintf("attest pullrequest github --name .foo --github-org myorg --commit %s --repository myrepo %s", suite.commitWithPR, suite.defaultKosliArguments), golden: "Error: failed to parse attestation name: invalid attestation name format: .foo\n", }, } @@ -172,8 +181,6 @@ func (suite *AttestGithubPRCommandTestSuite) TestAttestGithubPRCmd() { runTestCmd(suite.T(), tests) } -// In order for 'go test' to run this suite, we need to create -// a normal test function and pass our suite to suite.Run func TestAttestGithubPRCommandTestSuite(t *testing.T) { suite.Run(t, new(AttestGithubPRCommandTestSuite)) } diff --git a/cmd/kosli/pullrequest.go b/cmd/kosli/pullrequest.go index 72cae3b2c..8766a760f 100644 --- a/cmd/kosli/pullrequest.go +++ b/cmd/kosli/pullrequest.go @@ -5,12 +5,7 @@ import ( "net/http" "net/url" "os" - "reflect" - azUtils "github.com/kosli-dev/cli/internal/azure" - bbUtils "github.com/kosli-dev/cli/internal/bitbucket" - ghUtils "github.com/kosli-dev/cli/internal/github" - gitlabUtils "github.com/kosli-dev/cli/internal/gitlab" "github.com/kosli-dev/cli/internal/requests" "github.com/kosli-dev/cli/internal/types" ) @@ -44,7 +39,7 @@ func (o *attestPROptions) run(args []string) error { } label := "" - o.payload.GitProvider, label = getGitProviderAndLabel(o.retriever) + o.payload.GitProvider, label = o.getRetriever().ProviderAndLabel() var pullRequestsEvidence []*types.PREvidence pullRequestsEvidence, err = o.getRetriever().PREvidenceForCommitV2(o.payload.Commit.Sha1) @@ -90,21 +85,3 @@ func (o *attestPROptions) run(args []string) error { return wrapAttestationError(err) } - -func getGitProviderAndLabel(retriever any) (string, string) { - label := "pull request" - provider := "" - t := reflect.TypeOf(retriever) - switch t { - case reflect.TypeOf(&gitlabUtils.GitlabConfig{}): - provider = "gitlab" - label = "merge request" - case reflect.TypeOf(&ghUtils.GithubConfig{}): - provider = "github" - case reflect.TypeOf(&azUtils.AzureConfig{}): - provider = "azure" - case reflect.TypeOf(&bbUtils.Config{}): - provider = "bitbucket" - } - return provider, label -} diff --git a/internal/azure/azure.go b/internal/azure/azure.go index b0592abb7..67250c02d 100644 --- a/internal/azure/azure.go +++ b/internal/azure/azure.go @@ -55,6 +55,10 @@ func NewAzureClientFromToken(ctx context.Context, azToken, orgURL string) (git.C return gitClient, nil } +func (c *AzureConfig) ProviderAndLabel() (string, string) { + return "azure", "pull request" +} + // This is the old implementation, it will be removed after the PR payload is enhanced for Azure func (c *AzureConfig) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, error) { pullRequestsEvidence := []*types.PREvidence{} diff --git a/internal/bitbucket/bitbucket.go b/internal/bitbucket/bitbucket.go index 03d0e6772..73e2cb7ba 100644 --- a/internal/bitbucket/bitbucket.go +++ b/internal/bitbucket/bitbucket.go @@ -34,6 +34,10 @@ func parseRFC3339NanoTimestamp(timestampStr, fieldName string) (int64, error) { return parsedTime.Unix(), nil } +func (c *Config) ProviderAndLabel() (string, string) { + return "bitbucket", "pull request" +} + // This is the old implementation, it will be removed after the PR payload is enhanced for Bitbucket func (c *Config) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, error) { return c.getPullRequestsFromBitbucketApi(commit, 1) diff --git a/internal/github/fake_github.go b/internal/github/fake_github.go new file mode 100644 index 000000000..bac828b0c --- /dev/null +++ b/internal/github/fake_github.go @@ -0,0 +1,53 @@ +package github + +import ( + "errors" + "fmt" + + "github.com/kosli-dev/cli/internal/types" +) + +// errInjected is returned by FakeGitHubClient when Err is set. +var errInjected = errors.New("injected error") + +// FakeGitHubClient is an in-memory implementation of types.PRRetriever for +// testing. Seed PRsByCommit with the commits and PR evidence you want returned. +// Set Err to simulate a network or API failure. +type FakeGitHubClient struct { + // PRsByCommit maps a commit SHA to the PR evidence returned for that commit. + PRsByCommit map[string][]*types.PREvidence + // Err, if set, is returned by all calls regardless of commit. + Err error +} + +func (f *FakeGitHubClient) ProviderAndLabel() (string, string) { + return "github", "pull request" +} + +// PREvidenceForCommitV1 mirrors the REST API: returns an error for commits +// not present in PRsByCommit (matching the real GitHub V1 behaviour of +// returning 422 for unknown commits). +func (f *FakeGitHubClient) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, error) { + if f.Err != nil { + return nil, f.Err + } + prs, ok := f.PRsByCommit[commit] + if !ok { + return nil, fmt.Errorf("commit not found: %s", commit) + } + return prs, nil +} + +// PREvidenceForCommitV2 mirrors the GraphQL API: returns empty with no error +// for commits not present in PRsByCommit (matching the real GitHub V2 behaviour +// of returning null for unknown commits). +func (f *FakeGitHubClient) PREvidenceForCommitV2(commit string) ([]*types.PREvidence, error) { + if f.Err != nil { + return nil, f.Err + } + prs := f.PRsByCommit[commit] + if prs == nil { + return []*types.PREvidence{}, nil + } + return prs, nil +} diff --git a/internal/github/github.go b/internal/github/github.go index 36a4aee01..b450cf6c5 100644 --- a/internal/github/github.go +++ b/internal/github/github.go @@ -73,6 +73,23 @@ func graphqlEndpoint(baseURL string) string { return result } +func (c *GithubConfig) ProviderAndLabel() (string, string) { + return "github", "pull request" +} + +// NewGithubRetrieverFunc creates a types.PRRetriever from GitHub config +// parameters. It can be replaced in tests to inject a FakeGitHubClient. +var NewGithubRetrieverFunc = defaultNewGithubRetriever + +func defaultNewGithubRetriever(token, baseURL, org, repository string) types.PRRetriever { + return NewGithubConfig(token, baseURL, org, repository) +} + +// ResetGithubRetrieverFunc restores NewGithubRetrieverFunc to its default. +func ResetGithubRetrieverFunc() { + NewGithubRetrieverFunc = defaultNewGithubRetriever +} + func (c *GithubConfig) PREvidenceForCommitV2(commit string) ([]*types.PREvidence, error) { ctx := context.Background() pullRequestsEvidence := []*types.PREvidence{} diff --git a/internal/github/github_contract_test.go b/internal/github/github_contract_test.go new file mode 100644 index 000000000..e55921d0c --- /dev/null +++ b/internal/github/github_contract_test.go @@ -0,0 +1,111 @@ +package github + +import ( + "os" + "testing" + + "github.com/kosli-dev/cli/internal/testHelpers" + "github.com/kosli-dev/cli/internal/types" + "github.com/stretchr/testify/require" +) + +// runGitHubContractTests exercises the types.PRRetriever contract against any +// implementation. commitWithPR must be a commit SHA that has at least one +// associated pull request. commitUnknown must be a validly-formatted SHA that +// does not exist in the repository. +// +// V1 and V2 have different contracts for unknown commits: +// - V2 (GraphQL) returns empty with no error — the GraphQL API returns null +// for objects that don't exist. +// - V1 (REST) returns an error — the REST API returns 422 for unknown commits. +// +// Any implementation that passes this suite is a valid stand-in for the real +// GitHub API as far as this codebase is concerned. +func runGitHubContractTests(t *testing.T, provider types.PRRetriever, commitWithPR, commitUnknown string) { + t.Helper() + + t.Run("V2 returns PRs for commit with PRs", func(t *testing.T) { + prs, err := provider.PREvidenceForCommitV2(commitWithPR) + require.NoError(t, err) + require.NotEmpty(t, prs) + require.NotEmpty(t, prs[0].URL, "URL should be present") + require.NotEmpty(t, prs[0].State, "State should be present") + require.Equal(t, commitWithPR, prs[0].MergeCommit, "V2 sets MergeCommit to the queried commit SHA") + }) + + t.Run("V2 returns empty with no error for unknown commit", func(t *testing.T) { + prs, err := provider.PREvidenceForCommitV2(commitUnknown) + require.NoError(t, err) + require.Empty(t, prs) + }) + + t.Run("V1 returns PRs for commit with PRs", func(t *testing.T) { + prs, err := provider.PREvidenceForCommitV1(commitWithPR) + require.NoError(t, err) + require.NotEmpty(t, prs) + require.NotEmpty(t, prs[0].URL, "URL should be present") + require.NotEmpty(t, prs[0].State, "State should be present") + require.NotEmpty(t, prs[0].MergeCommit, "MergeCommit should be present") + }) + + t.Run("V1 returns error for unknown commit", func(t *testing.T) { + _, err := provider.PREvidenceForCommitV1(commitUnknown) + require.Error(t, err) + }) + + t.Run("ProviderAndLabel returns github and pull request", func(t *testing.T) { + providerName, label := provider.ProviderAndLabel() + require.Equal(t, "github", providerName) + require.Equal(t, "pull request", label) + }) +} + +func TestGitHubContract_Fake(t *testing.T) { + commitWithPR := "abc123" + commitUnknown := "0000000000000000000000000000000000000000" + + pr := &types.PREvidence{ + URL: "https://github.com/kosli-dev/cli/pull/1", + State: "MERGED", + MergeCommit: commitWithPR, + } + + client := &FakeGitHubClient{ + PRsByCommit: map[string][]*types.PREvidence{ + commitWithPR: {pr}, + }, + } + + runGitHubContractTests(t, client, commitWithPR, commitUnknown) + + // Error injection is a fake-specific mechanism with no real-API equivalent. + // These tests verify the fake itself, not the contract. + t.Run("V2 returns error when Err is injected", func(t *testing.T) { + client.Err = errInjected + defer func() { client.Err = nil }() + _, err := client.PREvidenceForCommitV2(commitWithPR) + require.Error(t, err) + }) + + t.Run("V1 returns error when Err is injected", func(t *testing.T) { + client.Err = errInjected + defer func() { client.Err = nil }() + _, err := client.PREvidenceForCommitV1(commitWithPR) + require.Error(t, err) + }) +} + +func TestGitHubContract_RealGitHub(t *testing.T) { + testHelpers.SkipIfEnvVarUnset(t, []string{"KOSLI_GITHUB_TOKEN"}) + + config := NewGithubConfig( + os.Getenv("KOSLI_GITHUB_TOKEN"), + "", + "kosli-dev", + "cli", + ) + + // commitUnknown is a validly-formatted SHA that does not exist in kosli-dev/cli. + commitUnknown := "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + runGitHubContractTests(t, config, testHelpers.GithubCommitWithPR(), commitUnknown) +} diff --git a/internal/gitlab/gitlab.go b/internal/gitlab/gitlab.go index c5bad0dab..bdb7bae97 100644 --- a/internal/gitlab/gitlab.go +++ b/internal/gitlab/gitlab.go @@ -38,6 +38,10 @@ func (c *GitlabConfig) ProjectID() string { return fmt.Sprintf("%s/%s", c.Org, c.Repository) } +func (c *GitlabConfig) ProviderAndLabel() (string, string) { + return "gitlab", "merge request" +} + // This is the old implementation, it will be removed after the PR payload is enhanced for all VCS providers func (c *GitlabConfig) PREvidenceForCommitV1(commit string) ([]*types.PREvidence, error) { pullRequestsEvidence := []*types.PREvidence{} diff --git a/internal/types/types.go b/internal/types/types.go index e80d81646..e4d3dd889 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -32,4 +32,7 @@ type Commit struct { type PRRetriever interface { PREvidenceForCommitV2(string) ([]*PREvidence, error) PREvidenceForCommitV1(string) ([]*PREvidence, error) + // ProviderAndLabel returns the provider name (e.g. "github") and the label + // for a pull request (e.g. "pull request", or "merge request" for GitLab). + ProviderAndLabel() (string, string) }