Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use setters and getters to avoid race condition #262

Merged
merged 1 commit into from
Feb 5, 2018
Merged

Conversation

kirederik
Copy link
Collaborator

when accessing ghttp.AllowUnhandledRequests and ghttp.UnhandledRequestsStatusCode

Solves #173

with @edwardecook

when accessing ghttp.AllowUnhandledRequests and ghttp.UnhandledRequestsStatusCode

Solves #173

Signed-off-by: Derik Evangelista <devangelista@pivotal.io>
@williammartin
Copy link
Sponsor Collaborator

Awesome, thanks! Will try to review tomorrow.

@williammartin
Copy link
Sponsor Collaborator

After modifying my racy test to use the Setters, things are passing:

var _ = Describe("Ghttprace", func() {

	var server *ghttp.Server
	var wg *sync.WaitGroup

	BeforeEach(func() {
		server = ghttp.NewServer()
		server.UnhandledRequestStatusCode = 421
		wg = &sync.WaitGroup{}
	})

	AfterEach(func() {
		server.Close()
	})

	JustBeforeEach(func() {
		wg.Add(100)
		go func(group *sync.WaitGroup) {
			for i := 0; i < 100; i++ {
				_, err := http.Get(server.URL())
				Expect(err).NotTo(HaveOccurred())
				group.Done()
			}
		}(wg)
	})

	Context("setting up some server stuff", func() {
		It("races", func() {
			server.AllowUnhandledRequests = true
			wg.Wait()
		})
	})
})

--->

var _ = Describe("Ghttprace", func() {

	var server *ghttp.Server
	var wg *sync.WaitGroup

	BeforeEach(func() {
		server = ghttp.NewServer()
		server.SetUnhandledRequestStatusCode(421)
		wg = &sync.WaitGroup{}
	})

	AfterEach(func() {
		server.Close()
	})

	JustBeforeEach(func() {
		wg.Add(100)
		go func(group *sync.WaitGroup) {
			for i := 0; i < 100; i++ {
				_, err := http.Get(server.URL())
				Expect(err).NotTo(HaveOccurred())
				group.Done()
			}
		}(wg)
	})

	Context("setting up some server stuff", func() {
		It("races", func() {
			server.SetAllowUnhandledRequests(true)
			wg.Wait()
		})
	})
})

LGTM, thanks.

@williammartin williammartin merged commit a9c79f1 into master Feb 5, 2018
@williammartin
Copy link
Sponsor Collaborator

@kirederik @edwardecook Any chance of a PR to fix up the Gomega docs, to suggest using the Getters/Setters here (https://onsi.github.io/gomega/) e.g.

It is sometimes useful to have a fake server that simply returns a fixed status code for all unhandled incoming requests. ghttp supports this: just set server.AllowUnhandledRequests = true and server.UnhandledRequestStatusCode to whatever status code you’d like to return.

and some others...

The branch is gh-pages on this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants