[BUG] Make delay writer actually work

- Reading the code of this delay writer implemenation, it looks like
that it should only actually write content to the `io.Writer` if x
amount of time has passed by. However in practice it was always printing
the buffer even if the X amount of time didn't pass yet. This is in line
with what was being said in the issue that this was to help with
https://github.com/go-gitea/gitea/issues/9610.
- This was caused by the extra `Close()` calls which in turn caused that
when the second `Close` is called (which is done in a defer already) it
would've printed the buffer anyway. So remove the extra calls to `Close()`.
- Add unit test.

(cherry picked from commit 9320ffd2b5)
This commit is contained in:
Gusted 2024-04-03 01:17:25 +02:00 committed by GitHub
parent c0fb79b436
commit 71e63f541d
2 changed files with 81 additions and 7 deletions

View file

@ -347,11 +347,10 @@ Forgejo or set your environment appropriately.`, "")
} }
var out io.Writer var out io.Writer
var dWriter *delayWriter
out = &nilWriter{} out = &nilWriter{}
if setting.Git.VerbosePush { if setting.Git.VerbosePush {
if setting.Git.VerbosePushDelay > 0 { if setting.Git.VerbosePushDelay > 0 {
dWriter = newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay) dWriter := newDelayWriter(os.Stdout, setting.Git.VerbosePushDelay)
defer dWriter.Close() defer dWriter.Close()
out = dWriter out = dWriter
} else { } else {
@ -414,7 +413,6 @@ Forgejo or set your environment appropriately.`, "")
hookOptions.RefFullNames = refFullNames hookOptions.RefFullNames = refFullNames
resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
if extra.HasError() { if extra.HasError() {
_ = dWriter.Close()
hookPrintResults(results) hookPrintResults(results)
return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error) return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error)
} }
@ -434,7 +432,6 @@ Forgejo or set your environment appropriately.`, "")
} }
fmt.Fprintf(out, "Processed %d references in total\n", total) fmt.Fprintf(out, "Processed %d references in total\n", total)
_ = dWriter.Close()
hookPrintResults(results) hookPrintResults(results)
return nil return nil
} }
@ -447,7 +444,6 @@ Forgejo or set your environment appropriately.`, "")
resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions) resp, extra := private.HookPostReceive(ctx, repoUser, repoName, hookOptions)
if resp == nil { if resp == nil {
_ = dWriter.Close()
hookPrintResults(results) hookPrintResults(results)
return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error) return fail(ctx, extra.UserMsg, "HookPostReceive failed: %v", extra.Error)
} }
@ -463,9 +459,8 @@ Forgejo or set your environment appropriately.`, "")
return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error) return fail(ctx, extra.UserMsg, "SetDefaultBranch failed: %v", extra.Error)
} }
} }
_ = dWriter.Close()
hookPrintResults(results)
hookPrintResults(results)
return nil return nil
} }

View file

@ -7,10 +7,20 @@ import (
"bufio" "bufio"
"bytes" "bytes"
"context" "context"
"io"
"net/http"
"net/http/httptest"
"os"
"strings" "strings"
"testing" "testing"
"time"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/test"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/urfave/cli/v2"
) )
func TestPktLine(t *testing.T) { func TestPktLine(t *testing.T) {
@ -83,3 +93,72 @@ func TestPktLine(t *testing.T) {
assert.Empty(t, w.Bytes()) assert.Empty(t, w.Bytes())
}) })
} }
func TestDelayWriter(t *testing.T) {
// Setup the environment.
defer test.MockVariableValue(&setting.InternalToken, "Random")()
defer test.MockVariableValue(&setting.InstallLock, true)()
defer test.MockVariableValue(&setting.Git.VerbosePush, true)()
require.NoError(t, os.Setenv("SSH_ORIGINAL_COMMAND", "true"))
// Setup the Stdin.
f, err := os.OpenFile(t.TempDir()+"/stdin", os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666)
require.NoError(t, err)
_, err = f.Write([]byte("00000000000000000000 00000000000000000001 refs/head/main\n"))
require.NoError(t, err)
_, err = f.Seek(0, 0)
require.NoError(t, err)
defer test.MockVariableValue(os.Stdin, *f)()
// Setup the server that processes the hooks.
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
time.Sleep(time.Millisecond * 600)
}))
defer ts.Close()
defer test.MockVariableValue(&setting.LocalURL, ts.URL+"/")()
app := cli.NewApp()
app.Commands = []*cli.Command{subcmdHookPreReceive}
// Capture what's being written into stdout
captureStdout := func(t *testing.T) (finish func() (output string)) {
t.Helper()
r, w, err := os.Pipe()
require.NoError(t, err)
resetStdout := test.MockVariableValue(os.Stdout, *w)
return func() (output string) {
w.Close()
resetStdout()
out, err := io.ReadAll(r)
require.NoError(t, err)
return string(out)
}
}
t.Run("Should delay", func(t *testing.T) {
defer test.MockVariableValue(&setting.Git.VerbosePushDelay, time.Millisecond*500)()
finish := captureStdout(t)
err = app.Run([]string{"./forgejo", "pre-receive"})
require.NoError(t, err)
out := finish()
require.Contains(t, out, "* Checking 1 references")
require.Contains(t, out, "Checked 1 references in total")
})
t.Run("Shouldn't delay", func(t *testing.T) {
defer test.MockVariableValue(&setting.Git.VerbosePushDelay, time.Second*5)()
finish := captureStdout(t)
err = app.Run([]string{"./forgejo", "pre-receive"})
require.NoError(t, err)
out := finish()
require.NoError(t, err)
require.Empty(t, out)
})
}