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

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

Merged
merged 1 commit into from
Feb 13, 2018

Conversation

nodo
Copy link
Collaborator

@nodo nodo commented Feb 12, 2018

Fixes #426

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

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

@nodo
Copy link
Collaborator Author

nodo commented Feb 12, 2018

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
Copy link
Collaborator Author

nodo commented Feb 13, 2018

@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
Sponsor 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.

None yet

2 participants