Skip to content
This repository was archived by the owner on Apr 15, 2025. It is now read-only.

Commit e199cc1

Browse files
committed
Properly (re-)initialize config subsystem during the tests
In some unit tests, we need to alter the configuration via env var (to ensure the environment-based changes are effective); this adds in re-initialization at the appropriate points. Additionally, this moves the various timeout constants to the config subsystem, allowing us to use much shorter timeouts for test runs (taking about 6 minutes off the time to run `go test`).
1 parent 9998833 commit e199cc1

File tree

4 files changed

+57
-10
lines changed

4 files changed

+57
-10
lines changed

config/config.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
package config
33

44
import (
5-
"path"
5+
"path/filepath"
66
"strconv"
77
"strings"
88
"os"
@@ -43,7 +43,7 @@ type OSDFConfig struct {
4343
// need to manually configure the location of the director endpoint.
4444
//
4545
func GetPreferredPrefix() string {
46-
arg0 := strings.ToUpper(path.Base(os.Args[0]))
46+
arg0 := strings.ToUpper(filepath.Base(os.Args[0]))
4747
underscore_idx := strings.Index(arg0, "_")
4848
if underscore_idx != -1 {
4949
return arg0[0:underscore_idx]
@@ -76,8 +76,12 @@ func Init() error {
7676
upper_prefix := GetPreferredPrefix()
7777
lower_prefix := strings.ToLower(upper_prefix)
7878

79+
viper.SetDefault("StoppedTransferTimeout", 100)
80+
viper.SetDefault("SlowTransferRampupTime", 100)
81+
viper.SetDefault("SlowTransferWindow", 30)
82+
7983
if upper_prefix == "OSDF" || upper_prefix == "STASH" {
80-
viper.Set("NamespaceURL", "https://topology.opensciencegrid.org/stashcache/namespaces")
84+
viper.SetDefault("NamespaceURL", "https://topology.opensciencegrid.org/stashcache/namespaces")
8185
}
8286

8387
viper.SetEnvPrefix(upper_prefix)

handle_http.go

+10-7
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,9 @@ func DownloadHTTP(transfer TransferDetails, dest string, token string) (int64, e
488488
)
489489
}
490490

491+
stoppedTransferTimeout := viper.GetInt64("StoppedTransferTimeout")
492+
slowTransferRampupTime := viper.GetInt64("SlowTransferRampupTime")
493+
slowTransferWindow := viper.GetInt64("SlowTransferWindow")
491494
var previousCompletedBytes int64 = 0
492495
var previousCompletedTime = time.Now()
493496
var startBelowLimit int64 = 0
@@ -514,8 +517,8 @@ Loop:
514517
if resp.BytesComplete() == lastBytesComplete {
515518
if noProgressStartTime.IsZero() {
516519
noProgressStartTime = time.Now()
517-
} else if time.Since(noProgressStartTime) > 100 * time.Second {
518-
errMsg := "No progress for more than " + (time.Since(noProgressStartTime)/time.Second).String() + " seconds."
520+
} else if time.Since(noProgressStartTime) > time.Duration(stoppedTransferTimeout) * time.Second {
521+
errMsg := "No progress for more than " + time.Since(noProgressStartTime).Truncate(time.Millisecond).String()
519522
log.Errorln(errMsg)
520523
return 5, &StoppedTransferError{
521524
Err: errMsg,
@@ -529,18 +532,18 @@ Loop:
529532

530533
// Check if we are downloading fast enough
531534
if resp.BytesPerSecond() < float64(downloadLimit) {
532-
// Give the download 120 seconds to start
533-
if resp.Duration() < time.Second*120 {
535+
// Give the download `slowTransferRampupTime` (default 120) seconds to start
536+
if resp.Duration() < time.Second* time.Duration(slowTransferRampupTime) {
534537
continue
535538
} else if startBelowLimit == 0 {
536539
log.Warnln("Download speed of ", resp.BytesPerSecond(), "bytes/s", " is below the limit of", downloadLimit, "bytes/s")
537540
startBelowLimit = time.Now().Unix()
538541
continue
539-
} else if (time.Now().Unix() - startBelowLimit) < 30 {
540-
// If the download is below the threshold for less than 30 seconds, continue
542+
} else if (time.Now().Unix() - startBelowLimit) < slowTransferWindow {
543+
// If the download is below the threshold for less than `SlowTransferWindow` (default 30) seconds, continue
541544
continue
542545
}
543-
// The download is below the threshold for more than 30 seconds, cancel the download
546+
// The download is below the threshold for more than `SlowTransferWindow` seconds, cancel the download
544547
cancel()
545548
if Options.ProgressBars {
546549
var cancelledProgressBar = p.AddBar(0,

handle_http_test.go

+24
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,30 @@ import (
55
"net"
66
"net/http"
77
"net/http/httptest"
8+
"net/http/httputil"
89
"net/url"
910
"os"
1011
"path/filepath"
1112
"strings"
1213
"testing"
1314
"time"
15+
1416
"github.com/stretchr/testify/assert"
17+
"github.com/spf13/viper"
1518

1619
"net/http/httputil"
1720

1821
namespaces "github.com/htcondor/osdf-client/v6/namespaces"
22+
"github.com/htcondor/osdf-client/v6/config"
1923
)
2024

25+
func TestMain(m *testing.M) {
26+
if err := config.Init(); err != nil {
27+
os.Exit(1)
28+
}
29+
os.Exit(m.Run())
30+
}
31+
2132
// TestIsPort calls main.hasPort with a hostname, checking
2233
// for a valid return value.
2334
func TestIsPort(t *testing.T) {
@@ -93,6 +104,8 @@ func TestNewTransferDetailsEnv(t *testing.T) {
93104
}
94105

95106
os.Setenv("OSG_DISABLE_PROXY_FALLBACK", "")
107+
err := config.Init()
108+
assert.Nil(t, err)
96109
transfers := NewTransferDetails(testCache, false)
97110
assert.Equal(t, 1, len(transfers))
98111
assert.Equal(t, true, transfers[0].Proxy)
@@ -102,9 +115,16 @@ func TestNewTransferDetailsEnv(t *testing.T) {
102115
assert.Equal(t, "https", transfers[0].Url.Scheme)
103116
assert.Equal(t, false, transfers[0].Proxy)
104117
os.Unsetenv("OSG_DISABLE_PROXY_FALLBACK")
118+
viper.Reset()
119+
err = config.Init()
120+
assert.Nil(t, err)
105121
}
106122

107123
func TestSlowTransfers(t *testing.T) {
124+
// Adjust down some timeouts to speed up the test
125+
viper.Set("SlowTransferWindow", 5)
126+
viper.Set("SlowTransferRampupTime", 10)
127+
108128
channel := make(chan bool)
109129
slowDownload := 1024 * 10 // 10 KiB/s < 100 KiB/s
110130
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -164,6 +184,10 @@ func TestSlowTransfers(t *testing.T) {
164184

165185
// Test stopped transfer
166186
func TestStoppedTransfer(t *testing.T) {
187+
// Adjust down the timeouts
188+
viper.Set("StoppedTransferTimeout", 3)
189+
viper.Set("SlowTransferRampupTime", 100)
190+
167191
channel := make(chan bool)
168192
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
169193
buffer := make([]byte, 1024 * 100)

namespaces/namespaces_test.go

+16
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import (
55
"testing"
66

77
"github.com/stretchr/testify/assert"
8+
"github.com/spf13/viper"
9+
10+
"github.com/htcondor/osdf-client/v6/config"
811
)
912

1013
// TestMatchNamespace calls MatchNamespace with a hostname, checking
@@ -68,6 +71,10 @@ func TestMatchNamespace(t *testing.T) {
6871
if err != nil {
6972
t.Error(err)
7073
}
74+
viper.Reset()
75+
err = config.Init()
76+
assert.Nil(t, err)
77+
7178
ns, err := MatchNamespace("/osgconnect/private/path/to/file.txt")
7279
assert.NoError(t, err, "Failed to parse namespace")
7380

@@ -150,6 +157,9 @@ func TestFullNamespace(t *testing.T) {
150157
// TestDownloadNamespaces tests the download of the namespaces JSON
151158
func TestDownloadNamespaces(t *testing.T) {
152159
os.Setenv("STASH_NAMESPACE_URL", "https://topology-itb.opensciencegrid.org/stashcache/namespaces")
160+
viper.Reset()
161+
err := config.Init()
162+
assert.Nil(t, err)
153163
defer os.Unsetenv("STASH_NAMESPACE_URL")
154164
namespaceBytes, err := downloadNamespace()
155165
assert.NoError(t, err, "Failed to download namespaces")
@@ -159,6 +169,9 @@ func TestDownloadNamespaces(t *testing.T) {
159169

160170
func TestDownloadNamespacesFail(t *testing.T) {
161171
os.Setenv("STASH_NAMESPACE_URL", "https://doesnotexist.org.blah/namespaces.json")
172+
viper.Reset()
173+
err := config.Init()
174+
assert.Nil(t, err)
162175
defer os.Unsetenv("STASH_NAMESPACE_URL")
163176
namespaceBytes, err := downloadNamespace()
164177
assert.Error(t, err, "Failed to download namespaces")
@@ -168,6 +181,9 @@ func TestDownloadNamespacesFail(t *testing.T) {
168181
func TestGetNamespaces(t *testing.T) {
169182
// Set the environment to an invalid URL, so it is forced to use the "built-in" namespaces.json
170183
os.Setenv("STASH_NAMESPACE_URL", "https://doesnotexist.org.blah/namespaces.json")
184+
viper.Reset()
185+
err := config.Init()
186+
assert.Nil(t, err)
171187
defer os.Unsetenv("STASH_NAMESPACE_URL")
172188
namespaces, err := GetNamespaces()
173189
assert.NoError(t, err, "Failed to get namespaces")

0 commit comments

Comments
 (0)