Skip to content

Commit c27ec73

Browse files
bsardobsardo
authored
Rework pubstack module tests to remove race conditions (prebid#1522)
* Rework pubstack module tests to remove race conditions * PR feedback * Remove event count and add helper methods to assert events received on channel
1 parent 7feefad commit c27ec73

File tree

1 file changed

+162
-123
lines changed

1 file changed

+162
-123
lines changed

‎analytics/pubstack/pubstack_module_test.go

Lines changed: 162 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ package pubstack
22

33
import (
44
"encoding/json"
5-
"github.com/prebid/prebid-server/analytics/pubstack/eventchannel"
65
"io/ioutil"
76
"net/http"
87
"net/http/httptest"
9-
"net/url"
108
"os"
119
"testing"
1210
"time"
@@ -16,7 +14,7 @@ import (
1614
"github.com/stretchr/testify/assert"
1715
)
1816

19-
func loadJsonFromFile() (*analytics.AuctionObject, error) {
17+
func loadJSONFromFile() (*analytics.AuctionObject, error) {
2018
req, err := os.Open("mocks/mock_openrtb_request.json")
2119
if err != nil {
2220
return nil, err
@@ -57,130 +55,171 @@ func loadJsonFromFile() (*analytics.AuctionObject, error) {
5755
}, nil
5856
}
5957

60-
func TestPubstackModule(t *testing.T) {
61-
62-
remoteConfig := &Configuration{}
63-
server := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) {
64-
defer req.Body.Close()
65-
data, _ := json.Marshal(remoteConfig)
66-
res.Write(data)
67-
}))
68-
client := server.Client()
69-
70-
defer server.Close()
71-
72-
// Loading Issues
73-
_, err := NewPubstackModule(client, "scope", server.URL, "1z", 100, "90MB", "15m")
74-
assert.NotNil(t, err) // should raise an error since we can't parse args // configRefreshDelay
75-
76-
_, err = NewPubstackModule(client, "scope", server.URL, "1h", 100, "90z", "15m")
77-
assert.NotNil(t, err) // should raise an error since we can't parse args // maxByte
78-
79-
_, err = NewPubstackModule(client, "scope", server.URL, "1h", 100, "90MB", "15z")
80-
assert.NotNil(t, err) // should raise an error since we can't parse args // maxTime
81-
82-
// Loading OK
83-
module, err := NewPubstackModule(client, "scope", server.URL, "10ms", 100, "90MB", "15m")
84-
assert.Nil(t, err)
85-
86-
// Default Configuration
87-
pubstack, ok := module.(*PubstackModule)
88-
assert.Equal(t, ok, true) //PBSAnalyticsModule is also a PubstackModule
89-
assert.Equal(t, len(pubstack.cfg.Features), 5)
90-
assert.Equal(t, pubstack.cfg.Features[auction], false)
91-
assert.Equal(t, pubstack.cfg.Features[video], false)
92-
assert.Equal(t, pubstack.cfg.Features[amp], false)
93-
assert.Equal(t, pubstack.cfg.Features[setUID], false)
94-
assert.Equal(t, pubstack.cfg.Features[cookieSync], false)
95-
96-
assert.Equal(t, len(pubstack.eventChannels), 0)
97-
98-
// Process Auction Event
99-
counter := 0
100-
send := func(_ []byte) error {
101-
counter++
102-
return nil
58+
func TestPubstackModuleErrors(t *testing.T) {
59+
tests := []struct {
60+
description string
61+
refreshDelay string
62+
maxByteSize string
63+
maxTime string
64+
}{
65+
{
66+
description: "refresh delay is in an invalid format",
67+
refreshDelay: "1invalid",
68+
maxByteSize: "90MB",
69+
maxTime: "15m",
70+
},
71+
{
72+
description: "max byte size is in an invalid format",
73+
refreshDelay: "1h",
74+
maxByteSize: "90invalid",
75+
maxTime: "15m",
76+
},
77+
{
78+
description: "max time is in an invalid format",
79+
refreshDelay: "1h",
80+
maxByteSize: "90MB",
81+
maxTime: "15invalid",
82+
},
10383
}
104-
mockedEvent, err := loadJsonFromFile()
105-
if err != nil {
106-
t.Fail()
84+
85+
for _, tt := range tests {
86+
_, err := NewPubstackModule(&http.Client{}, "scope", "http://example.com", tt.refreshDelay, 100, tt.maxByteSize, tt.maxTime)
87+
assert.NotNil(t, err, tt.description)
10788
}
89+
}
10890

109-
pubstack.eventChannels[auction] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
110-
pubstack.eventChannels[video] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
111-
pubstack.eventChannels[amp] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
112-
pubstack.eventChannels[setUID] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
113-
pubstack.eventChannels[cookieSync] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
114-
115-
pubstack.LogAuctionObject(mockedEvent)
116-
pubstack.LogAmpObject(&analytics.AmpObject{
117-
Status: http.StatusOK,
118-
})
119-
pubstack.LogCookieSyncObject(&analytics.CookieSyncObject{
120-
Status: http.StatusOK,
121-
})
122-
pubstack.LogVideoObject(&analytics.VideoObject{
123-
Status: http.StatusOK,
124-
})
125-
pubstack.LogSetUIDObject(&analytics.SetUIDObject{
126-
Status: http.StatusOK,
127-
})
128-
129-
pubstack.closeAllEventChannels()
130-
time.Sleep(10 * time.Millisecond) // process channel
131-
assert.Equal(t, counter, 0)
132-
133-
// Hot-Reload config
134-
newFeatures := make(map[string]bool)
135-
newFeatures[auction] = true
136-
newFeatures[video] = true
137-
newFeatures[amp] = true
138-
newFeatures[cookieSync] = true
139-
newFeatures[setUID] = true
140-
141-
remoteConfig = &Configuration{
142-
ScopeID: "new-scope",
143-
Endpoint: "new-endpoint",
144-
Features: newFeatures,
91+
func TestPubstackModuleSuccess(t *testing.T) {
92+
tests := []struct {
93+
description string
94+
feature string
95+
logObject func(analytics.PBSAnalyticsModule)
96+
}{
97+
{
98+
description: "auction events are only published when logging an auction object with auction feature on",
99+
feature: auction,
100+
logObject: func(module analytics.PBSAnalyticsModule) {
101+
module.LogAuctionObject(&analytics.AuctionObject{Status: http.StatusOK})
102+
},
103+
},
104+
{
105+
description: "AMP events are only published when logging an AMP object with AMP feature on",
106+
feature: amp,
107+
logObject: func(module analytics.PBSAnalyticsModule) {
108+
module.LogAmpObject(&analytics.AmpObject{Status: http.StatusOK})
109+
},
110+
},
111+
{
112+
description: "video events are only published when logging a video object with video feature on",
113+
feature: video,
114+
logObject: func(module analytics.PBSAnalyticsModule) {
115+
module.LogVideoObject(&analytics.VideoObject{Status: http.StatusOK})
116+
},
117+
},
118+
{
119+
description: "cookie events are only published when logging a cookie object with cookie feature on",
120+
feature: cookieSync,
121+
logObject: func(module analytics.PBSAnalyticsModule) {
122+
module.LogCookieSyncObject(&analytics.CookieSyncObject{Status: http.StatusOK})
123+
},
124+
},
125+
{
126+
description: "setUID events are only published when logging a setUID object with setUID feature on",
127+
feature: setUID,
128+
logObject: func(module analytics.PBSAnalyticsModule) {
129+
module.LogSetUIDObject(&analytics.SetUIDObject{Status: http.StatusOK})
130+
},
131+
},
145132
}
146133

147-
endpoint, _ := url.Parse(server.URL)
148-
pubstack.reloadConfig(endpoint)
149-
150-
time.Sleep(2 * time.Millisecond) // process channel
151-
assert.Equal(t, len(pubstack.cfg.Features), 5)
152-
assert.Equal(t, pubstack.cfg.Features[auction], true)
153-
assert.Equal(t, pubstack.cfg.Features[video], true)
154-
assert.Equal(t, pubstack.cfg.Features[amp], true)
155-
assert.Equal(t, pubstack.cfg.Features[setUID], true)
156-
assert.Equal(t, pubstack.cfg.Features[cookieSync], true)
157-
assert.Equal(t, pubstack.cfg.ScopeID, "new-scope")
158-
assert.Equal(t, pubstack.cfg.Endpoint, "new-endpoint")
159-
assert.Equal(t, len(pubstack.eventChannels), 5)
160-
161-
counter = 0
162-
pubstack.eventChannels[auction] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
163-
pubstack.eventChannels[video] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
164-
pubstack.eventChannels[amp] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
165-
pubstack.eventChannels[setUID] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
166-
pubstack.eventChannels[cookieSync] = eventchannel.NewEventChannel(send, 2000, 1, 10*time.Second)
167-
168-
pubstack.LogAuctionObject(mockedEvent)
169-
pubstack.LogAmpObject(&analytics.AmpObject{
170-
Status: http.StatusOK,
171-
})
172-
pubstack.LogCookieSyncObject(&analytics.CookieSyncObject{
173-
Status: http.StatusOK,
174-
})
175-
pubstack.LogVideoObject(&analytics.VideoObject{
176-
Status: http.StatusOK,
177-
})
178-
pubstack.LogSetUIDObject(&analytics.SetUIDObject{
179-
Status: http.StatusOK,
180-
})
181-
pubstack.closeAllEventChannels()
182-
time.Sleep(10 * time.Millisecond)
183-
184-
assert.Equal(t, counter, 5)
134+
for _, tt := range tests {
135+
// original config is loaded when the module is created
136+
// the feature is disabled so no events should be sent
137+
origConfig := &Configuration{
138+
Features: map[string]bool{
139+
tt.feature: false,
140+
},
141+
}
142+
// updated config is hot-reloaded after some time passes
143+
// the feature is enabled so events should be sent
144+
updatedConfig := &Configuration{
145+
Features: map[string]bool{
146+
tt.feature: true,
147+
},
148+
}
149+
150+
// create server with root endpoint that returns the current config
151+
// add an intake endpoint that PBS hits when events are sent
152+
rootCount := 0
153+
mux := http.NewServeMux()
154+
mux.HandleFunc("/", func(res http.ResponseWriter, req *http.Request) {
155+
rootCount++
156+
defer req.Body.Close()
157+
158+
if rootCount > 1 {
159+
if data, err := json.Marshal(updatedConfig); err != nil {
160+
res.WriteHeader(http.StatusBadRequest)
161+
} else {
162+
res.Write(data)
163+
}
164+
} else {
165+
if data, err := json.Marshal(origConfig); err != nil {
166+
res.WriteHeader(http.StatusBadRequest)
167+
} else {
168+
res.Write(data)
169+
}
170+
}
171+
})
172+
173+
intakeChannel := make(chan int) // using a channel rather than examining the count directly to avoid race
174+
mux.HandleFunc("/intake/"+tt.feature+"/", func(res http.ResponseWriter, req *http.Request) {
175+
intakeChannel <- 1
176+
})
177+
server := httptest.NewServer(mux)
178+
client := server.Client()
179+
180+
// set the server url on each of the configs
181+
origConfig.Endpoint = server.URL
182+
updatedConfig.Endpoint = server.URL
183+
184+
// instantiate module with 25ms config refresh rate
185+
module, err := NewPubstackModule(client, "scope", server.URL, "15ms", 100, "1B", "10ms")
186+
assert.Nil(t, err, tt.description)
187+
188+
// allow time for the module to load the original config
189+
time.Sleep(10 * time.Millisecond)
190+
191+
pubstack, _ := module.(*PubstackModule)
192+
// attempt to log but no event channel was created because the feature is disabled in the original config
193+
tt.logObject(pubstack)
194+
195+
// verify no event was received over a 10ms period
196+
assertChanNone(t, intakeChannel, tt.description)
197+
198+
// allow time for the server to start serving the updated config
199+
time.Sleep(10 * time.Millisecond)
200+
201+
// attempt to log; the event channel should have been created because the feature is enabled in updated config
202+
tt.logObject(pubstack)
203+
204+
// verify an event was received within 10ms
205+
assertChanOne(t, intakeChannel, tt.description)
206+
}
207+
}
185208

209+
func assertChanNone(t *testing.T, c <-chan int, msgAndArgs ...interface{}) bool {
210+
select {
211+
case <-c:
212+
return assert.Fail(t, "Should NOT receive an event, but did", msgAndArgs...)
213+
case <-time.After(10 * time.Millisecond):
214+
return true
215+
}
216+
}
217+
218+
func assertChanOne(t *testing.T, c <-chan int, msgAndArgs ...interface{}) bool {
219+
select {
220+
case <-c:
221+
return true
222+
case <-time.After(10 * time.Millisecond):
223+
return assert.Fail(t, "Should receive an event, but did NOT", msgAndArgs...)
224+
}
186225
}

0 commit comments

Comments
 (0)