[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.
This commit is contained in:
parent
68e900822b
commit
9320ffd2b5
2 changed files with 81 additions and 7 deletions
|
@ -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
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -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)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue