Skip to content
Snippets Groups Projects

Handle "git archive" via headers

Merged username-removed-5302 requested to merge archive-header into master

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 2 2
    3 3 import (
    4 4 "../helper"
    5 "encoding/base64"
    6 "encoding/json"
    5 "../senddata"
    7 6 "fmt"
    8 7 "io"
    9 8 "log"
    10 9 "net/http"
    11 "strings"
    12 10 )
    13 11
    14 type blobParams struct {
  • It is not directly related to this MR, but we should think about splitting the main_test.go in to separate files that tests various handlers.

  • I like the change. It makes sense to implement this that way 👍

    Edited by Kamil Trzcińśki
  • Added 2 commits:

    • f819a3c5 - Rename Handler to Injecter
    • 46680f42 - Configure sendfile and injecters in routes.go
  • Added 1 commit:

    • 28c9928c - Declare params structs outside Inject functions
  • @ayufan could you have another look?

  • 37 39 u.RoundTripper,
    38 40 )
    39 41 static := &staticpages.Static{u.DocumentRoot}
    40 proxy := proxypkg.NewProxy(
    41 u.Backend,
    42 u.Version,
    43 u.RoundTripper,
    42 proxy := senddata.SendData(
    43 sendfile.SendFile(proxypkg.NewProxy(
  • 659 659 })
    660 660 }
    661 661
    662 func archiveOKServer(t *testing.T, archiveName string) *httptest.Server {
    663 return testhelper.TestServerWithHandler(regexp.MustCompile("."), func(w http.ResponseWriter, r *http.Request) {
    • I think that we should have this test in: internal/archive/archive_test.go

    • Will that work OK with loading? Will give it a try.

    • @ayufan I am not sure if this is going to work.

      % git grep -n ArchiveOKServer
      internal/git/archive_test.go:15:func ArchiveOKServer(t *testing.T, repoPath, archiveName, cacheDir string) *httptest.Server {
      main_test.go:119:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)
      main_test.go:138:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)
      main_test.go:157:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)
      main_test.go:176:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)
      main_test.go:195:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)
      main_test.go:214:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)
      main_test.go:237:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)
      main_test.go:268:	ts := git.ArchiveOKServer(t, repoPath(t), archiveName, cacheDir)

      You can see above I moved the function into the 'git' package. But when I try to run the test:

      % support/path go test
      # _/Volumes/GitLab.com/gitlab-development-kit/gitlab-workhorse
      ./main_test.go:119: undefined: git.ArchiveOKServer
      ./main_test.go:138: undefined: git.ArchiveOKServer
      ./main_test.go:157: undefined: git.ArchiveOKServer
      ./main_test.go:176: undefined: git.ArchiveOKServer
      ./main_test.go:195: undefined: git.ArchiveOKServer
      ./main_test.go:214: undefined: git.ArchiveOKServer
      ./main_test.go:237: undefined: git.ArchiveOKServer
      ./main_test.go:268: undefined: git.ArchiveOKServer
      FAIL	_/Volumes/GitLab.com/gitlab-development-kit/gitlab-workhorse [build failed]
  • 659 659 })
    660 660 }
    661 661
    662 func archiveOKServer(t *testing.T, archiveName string) *httptest.Server {
    663 return testhelper.TestServerWithHandler(regexp.MustCompile("."), func(w http.ResponseWriter, r *http.Request) {
    664 cwd, err := os.Getwd()
    665 if err != nil {
    666 t.Fatal(err)
    667 }
    668 archivePath := path.Join(cwd, cacheDir, archiveName)
    669
    670 params := struct{ RepoPath, ArchivePath, CommitId, ArchivePrefix string }{
  • Small comments, overall it looks nice :)

  • Added 1 commit:

    • 92e88955 - Simplify WriteHeader in senddata
  • Added 22 commits:

  • Added 1 commit:

    • 85e6cbf8 - Use an early return in senddata.tryInject
  • @ayufan I am not going to spend more time trying to move that one test helper; I could not get it to work.

  • username-removed-5302 Title changed from WIP "git archive" via headers to Handle "git archive" via headers

    Title changed from WIP "git archive" via headers to Handle "git archive" via headers

  • username-removed-5302 Status changed to merged

    Status changed to merged

  • mentioned in commit 153527fb

  • @jacobvosmaer It's OK. My idea was to test the archive in it's own package and not put that in main.

  • Testing discussion: !39 (merged)

  • username-removed-5302 mentioned in merge request !39 (merged)

    mentioned in merge request !39 (merged)

  • Please register or sign in to reply
    Loading