Skip to content

Synchronise the access to the state of specs to avoid race conditions#430

Merged
williammartin merged 1 commit into
masterfrom
issue-426
Feb 13, 2018
Merged

Synchronise the access to the state of specs to avoid race conditions#430
williammartin merged 1 commit into
masterfrom
issue-426

Conversation

@nodo

@nodo nodo commented Feb 12, 2018

Copy link
Copy Markdown
Collaborator

Fixes #426

Signed-off-by: Ed Cook <ecook@pivotal.io>
Signed-off-by: Giuseppe Capizzi <gcapizzi@pivotal.io>
@williammartin

Copy link
Copy Markdown
Collaborator

LGTM but is there any way to get any test coverage out of this?

@nodo

nodo commented Feb 12, 2018

Copy link
Copy Markdown
Collaborator Author

Thanks @williammartin.

We were discussing it, it would be fairly easy to add an integration test for that. However, we were not sure about the value of this.

A possibility could be to add an integration test that:

  1. Build and start ginkgo
  2. Send a SIGINT
  3. Check that no panic occurred

Do you think that something like this would be good?

@nodo

nodo commented Feb 13, 2018

Copy link
Copy Markdown
Collaborator Author

@williammartin I have done a couple of tests with the idea above. The test looks a bit flakey to me and I wouldn't add it. Is that OK for you to merge the PR as it is?

@williammartin

Copy link
Copy Markdown
Collaborator

Yeh alright, just wanted to check to be sure. Thanks!

@williammartin williammartin merged commit ae6829d into master Feb 13, 2018
@nodo nodo deleted the issue-426 branch February 25, 2018 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants