From 788f3b9b82d7c54170d446f11953213fbddd3477 Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Wed, 28 Aug 2024 16:47:37 +0400 Subject: [PATCH] Pass `GITHUB_TOKEN` to tools tests (#4558) --- .github/workflows/go.yml | 2 + CONTRIBUTING.md | 4 +- Taskfile.yml | 2 +- tools/Taskfile.yml | 19 +++-- tools/checkcomments/checkcomments_test.go | 4 + tools/checkdocs/checkdocs.go | 2 +- tools/checkdocs/checkdocs_test.go | 12 ++- tools/cleantool/cleantool.go | 92 ----------------------- tools/github/client.go | 7 +- tools/github/client_test.go | 4 + tools/go.mod | 2 +- 11 files changed, 43 insertions(+), 107 deletions(-) delete mode 100644 tools/cleantool/cleantool.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 77891152fda7..ca3cd8a82e39 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -60,6 +60,7 @@ jobs: run: bin/task test-unit-short TEST_TIMEOUT=10m env: GOFLAGS: ${{ runner.debug == '1' && '-v' || '' }} + GITHUB_TOKEN: invalid # to test that -short is handled correctly # we don't want them on CI - name: Clean test and fuzz caches @@ -119,6 +120,7 @@ jobs: run: bin/task test-unit TEST_TIMEOUT=10m env: GOFLAGS: ${{ runner.debug == '1' && '-v' || '' }} + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # for tools # The token is not required but should make uploads more stable. # If secrets are unavailable (for example, for a pull request from a fork), it fallbacks to the tokenless uploads. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1d2e81fd08a7..67147f30d252 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -83,10 +83,10 @@ The results will be saved `tmp/bin`. Some of our development tools require access to public information on GitHub at a rate higher than allowed for unauthenticated requests. Those tools will report a problem in this case. -It could be solved by creating a new classic or fine-graned personal access token +It could be solved by creating a new classic or fine-grained personal access token [there](https://github.com/settings/tokens). No scopes are needed for classic tokens, not even `public_repo`. -For fine-graned tokens, only read-only access to public repositories is needed without any additional permissions. +For fine-grained tokens, only read-only access to public repositories is needed without any additional permissions. After generating a token, set the `GITHUB_TOKEN` environment variable: ```sh diff --git a/Taskfile.yml b/Taskfile.yml index 5099bf27a75b..f0fe669713e8 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -174,7 +174,7 @@ tasks: desc: "Run short unit tests (with caching)" cmds: - go test -short -timeout={{.TEST_TIMEOUT}} {{.RACE_FLAG}} -tags={{.BUILD_TAGS}} -shuffle=on -coverpkg=./... -coverprofile=cover.txt ./... - - bin/task{{exeExt}} -d tools tools-test + - bin/task{{exeExt}} -d tools tools-test-short test-unit: desc: "Run all unit tests" diff --git a/tools/Taskfile.yml b/tools/Taskfile.yml index 37d7a1e708ef..004ff95ce0d8 100644 --- a/tools/Taskfile.yml +++ b/tools/Taskfile.yml @@ -3,21 +3,28 @@ version: 3 vars: + PACKAGES: ./checkcomments/... ./checkdocs/... ./checkswitch/... ./github/... RACE_FLAG: -race={{and (ne OS "windows") (ne ARCH "arm") (ne ARCH "riscv64")}} tasks: + tools-test-short: + cmds: + - ../bin/envtool{{exeExt}} shell rmdir ../tmp/githubcache + - ../bin/envtool{{exeExt}} shell mkdir ../tmp/githubcache + - go test -short {{.RACE_FLAG}} -shuffle=on -coverprofile=cover.txt -coverpkg=./... {{.PACKAGES}} + tools-test: cmds: - ../bin/envtool{{exeExt}} shell rmdir ../tmp/githubcache - ../bin/envtool{{exeExt}} shell mkdir ../tmp/githubcache - - go test -short {{.RACE_FLAG}} -shuffle=on -coverprofile=cover.txt -coverpkg=./... ./checkcomments/... ./checkdocs/... ./checkswitch/... + - go test -count=1 {{.RACE_FLAG}} -shuffle=on -coverprofile=cover.txt -coverpkg=./... {{.PACKAGES}} lint: desc: "Run linters" cmds: - - ../bin/golangci-lint{{exeExt}} run --config=../.golangci.yml ./checkcomments/... ./checkdocs/... ./checkswitch/... - - ../bin/golangci-lint{{exeExt}} run --config=../.golangci-new.yml ./checkcomments/... ./checkdocs/... ./checkswitch/... - - ../bin/go-consistent{{exeExt}} -pedantic ./checkcomments/... ./checkdocs/... ./checkswitch/... + - ../bin/golangci-lint{{exeExt}} run --config=../.golangci.yml {{.PACKAGES}} + - ../bin/golangci-lint{{exeExt}} run --config=../.golangci-new.yml {{.PACKAGES}} + - ../bin/go-consistent{{exeExt}} -pedantic {{.PACKAGES}} - - go vet -vettool=../bin/checkswitch{{exeExt}} ./checkcomments/... ./checkdocs/... ./checkswitch/... - - go vet -vettool=../bin/checkcomments{{exeExt}} ./checkcomments/... ./checkdocs/... ./checkswitch/... + - go vet -vettool=../bin/checkswitch{{exeExt}} {{.PACKAGES}} + - go vet -vettool=../bin/checkcomments{{exeExt}} {{.PACKAGES}} diff --git a/tools/checkcomments/checkcomments_test.go b/tools/checkcomments/checkcomments_test.go index 3c117c377438..c850d21c062c 100644 --- a/tools/checkcomments/checkcomments_test.go +++ b/tools/checkcomments/checkcomments_test.go @@ -26,6 +26,10 @@ import ( ) func TestCheckCommentIssue(t *testing.T) { + if testing.Short() { + t.Skip("skipping in -short mode") + } + t.Parallel() path, err := github.CacheFilePath() diff --git a/tools/checkdocs/checkdocs.go b/tools/checkdocs/checkdocs.go index 63dd9cbc0538..8e120516dc23 100644 --- a/tools/checkdocs/checkdocs.go +++ b/tools/checkdocs/checkdocs.go @@ -277,7 +277,7 @@ func checkSupportedCommands(file string) error { } if failed { - return fmt.Errorf("supported commands table is not formated correctly") + return fmt.Errorf("supported commands table is not formatted correctly") } return nil diff --git a/tools/checkdocs/checkdocs_test.go b/tools/checkdocs/checkdocs_test.go index 170324da7312..eb61a7ba8c75 100644 --- a/tools/checkdocs/checkdocs_test.go +++ b/tools/checkdocs/checkdocs_test.go @@ -29,6 +29,10 @@ import ( ) func TestReal(t *testing.T) { + if testing.Short() { + t.Skip("skipping in -short mode") + } + blogFiles, err := filepath.Glob(filepath.Join("..", "..", "website", "blog", "*.md")) if err != nil { log.Fatal(err) @@ -89,8 +93,12 @@ func TestVerifyTruncateString(t *testing.T) { } func TestCheckSupportedCommands(t *testing.T) { - buf := new(bytes.Buffer) - l := log.New(buf, "", 0) + if testing.Short() { + t.Skip("skipping in -short mode") + } + + var buf bytes.Buffer + l := log.New(&buf, "", 0) p, err := github.CacheFilePath() require.NoError(t, err) diff --git a/tools/cleantool/cleantool.go b/tools/cleantool/cleantool.go deleted file mode 100644 index 5d18b12fba9c..000000000000 --- a/tools/cleantool/cleantool.go +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright 2021 FerretDB Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package main - -import ( - "context" - "fmt" - "log" - "os" - "strings" - "time" - - "github.com/google/go-github/v57/github" - "golang.org/x/oauth2" -) - -// if the most recent update time of the package was 90 days ago, then mark it as -// stale. -func isPackageToBeClean(p *github.PackageVersion) bool { - daysBack := 90 - toBeClean := false - - if time.Now().After(p.UpdatedAt.Add(time.Duration(daysBack) * 24 * time.Hour)) { - log.Printf("Stale version id: %v , recent update was at %s", p.GetID(), p.UpdatedAt) - - toBeClean = true - } else { - log.Printf("Skip version id: %v , recent update was at %s", p.GetID(), p.UpdatedAt) - } - - return toBeClean -} - -func main() { - ctx := context.Background() - tokenName := "ROBOT_TOKEN" - token := os.Getenv(tokenName) - - if token == "" { - log.Fatalf("env variable %v is not found, please set it before run it", tokenName) - } - - ts := oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: token}, - ) - tc := oauth2.NewClient(ctx, ts) - client := github.NewClient(tc) - packageType := "container" - packageName := "ferretdb-dev" - orgName := "FerretDB" - pageSize := 100 - pageIndex := 1 - - var versions []string - - for { - listOption := github.ListOptions{Page: pageIndex, PerPage: pageSize} - packageListOption := github.PackageListOptions{ListOptions: listOption} - packages, _, err := client.Organizations.PackageGetAllVersions(ctx, orgName, packageType, packageName, &packageListOption) - if err != nil { - log.Printf("Failed to get versions for page %d", pageIndex) - } - - for _, v := range packages { - if isPackageToBeClean(v) { - versions = append(versions, fmt.Sprintf("%v", v.GetID())) - } - } - - if len(packages) < pageSize { - log.Printf("Come to the last page (page %d)", pageIndex) - break - } - - pageIndex++ - } - - staleVersions := strings.Join(versions, ", ") - log.Printf("Found %d stale versions: %v", len(versions), staleVersions) -} diff --git a/tools/github/client.go b/tools/github/client.go index 5cbcb6cdda03..1c3a3ce1cc94 100644 --- a/tools/github/client.go +++ b/tools/github/client.go @@ -108,17 +108,18 @@ func CacheFilePath() (string, error) { } var ( - // urlRE represents correctly formated FerretDB issue. + // Correctly formatted FerretDB issue. // It returns repository name and the issue number as it's submatches. urlRE = regexp.MustCompile(`^\Qhttps://github.com/FerretDB/\E([-\w]+)/issues/(\d+)$`) // ErrIncorrectURL indicates that FerretDB issue URL is formatted incorrectly. ErrIncorrectURL = errors.New("invalid TODO: incorrect format") + // ErrIncorrectIssueNumber indicates that FerretDB issue number is formatted incorrectly. ErrIncorrectIssueNumber = errors.New("invalid TODO: incorrect issue number") ) -// parseIssueURL takes the properly formated FerretDB issue URL and returns it's +// parseIssueURL takes the properly formatted FerretDB issue URL and returns it's // repository name and issue number. // If the issue number or URL formatting is incorrect, the error is returned. func parseIssueURL(line string) (repo string, num int, err error) { @@ -256,6 +257,8 @@ func (c *Client) checkIssueStatus(ctx context.Context, repo string, num int) (Is switch s := IssueStatus(issue.GetState()); s { case IssueOpen, IssueClosed: return s, nil + case IssueNotFound: + fallthrough default: return "", fmt.Errorf("unknown issue state: %q", s) } diff --git a/tools/github/client_test.go b/tools/github/client_test.go index 424865d4c7de..fd725abe1cde 100644 --- a/tools/github/client_test.go +++ b/tools/github/client_test.go @@ -37,6 +37,10 @@ func TestCacheFilePath(t *testing.T) { } func TestClient(t *testing.T) { + if testing.Short() { + t.Skip("skipping in -short mode") + } + t.Parallel() cacheFilePath := filepath.Join(t.TempDir(), "cache.json") diff --git a/tools/go.mod b/tools/go.mod index 372241a1e5ea..d9c2c8c4b300 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -12,7 +12,6 @@ require ( github.com/quasilyte/go-consistent v0.6.1 github.com/rogpeppe/go-internal v1.12.0 github.com/stretchr/testify v1.9.0 - golang.org/x/oauth2 v0.21.0 golang.org/x/perf v0.0.0-20240707193131-dc66afd55b77 golang.org/x/pkgsite v0.0.0-20240701161620-30d9315975ff golang.org/x/tools v0.23.0 @@ -106,6 +105,7 @@ require ( golang.org/x/exp v0.0.0-20240707233637-46b078467d37 // indirect golang.org/x/mod v0.19.0 // indirect golang.org/x/net v0.27.0 // indirect + golang.org/x/oauth2 v0.21.0 // indirect golang.org/x/sync v0.7.0 // indirect golang.org/x/sys v0.22.0 // indirect golang.org/x/telemetry v0.0.0-20240522233618-39ace7a40ae7 // indirect