Allow anonymous acceptance tests without credentials - #3602
Conversation
|
👋 Hi, and thank you for this contribution! This repo is maintained by GitHub and community members on a best-effort basis. We'll get to this as soon as we can. You can help us prioritize by joining the discussion on open issues and PRs, sharing details on the changes you need, and reviewing other contributions. 🤖 This is an automated message. |
|
@rilical I'm not sure I agree with the design here or even the problem description, at first glance this change looks to add significant complexity which goes against to Go idiom of making code as readable as possible. The acceptance test design is to gate the tests, the helpers are expected to fail if they're used by a test that doesn't have permission as this is a coding error. FYI for a large change such as this it'd be best to open an issue to track the actual problem being solved rather than come with an implementation. |
262012c to
6d6ff28
Compare
|
@stevehipwell Would you mind giving this another look? |
robert-crandall
left a comment
There was a problem hiding this comment.
Thank you for simplifying this. This PR fixes a real problem of anonymous tests failing during setup.
There was a problem hiding this comment.
Pull request overview
These provider review instructions are being used. No blocking findings found.
Adds credential-free anonymous acceptance-test setup.
Changes:
- Skips credential configuration in anonymous mode.
- Adds regression coverage for anonymous metadata initialization.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
github/acc_test.go |
Bypasses credentials for anonymous tests. |
github/provider_test.go |
Adds anonymous-mode regression test. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
stevehipwell
left a comment
There was a problem hiding this comment.
The code change looks good to me, but we don't need the test.
|
@rilical if you can rebase your branch we can get this merged in. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Steve Hipwell <steve.hipwell@gmail.com>
524883b to
7043077
Compare
I found that
getTestMetastill attempted token or GitHub App credential setup whenGH_TEST_AUTH_MODE=anonymous, so credential-free anonymous acceptance tests exited during setup. This change skips credential setup only for anonymous mode, and the authenticated paths are unchanged.I added
Test_getTestMeta_anonymousDoesNotRequireCredentialsto cover the regression. I ranmake test,go vet ./..., andCGO_ENABLED=0 go build ./...;gofmt -l .returned no files.