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

Made request thread safe #289

Closed
wants to merge 1 commit into from

Conversation

Rohith-Raju
Copy link
Contributor

@Rohith-Raju Rohith-Raju commented Feb 9, 2023

Thread safety for request struct

  • Added lock and unlock methods
  • Added request_Reeterant test

Signed-off-by:_ ROHITH RAJU rohithraju488@gmail.com

@Rohith-Raju
Copy link
Contributor Author

@gavv your thoughts?

Signed-off-by: ROHITH RAJU <rohithraju488@gmail.com>
@coveralls
Copy link
Collaborator

coveralls commented Feb 9, 2023

Coverage Status

Coverage: 94.92% (+0.06%) from 94.859% when pulling 10f4df6 on Rohith-Raju:requestThreadSafety into c0c15b9 on gavv:master.

@gavv
Copy link
Owner

gavv commented Feb 9, 2023

Hey, thanks for PR, but seems you ignored the text from the issue? I mean the part about matchers and teansformers, which is essential part of it.

Look, I'm always happy to give an advice or help, but put yourself in my place. If I have to provide line-by-line guide each time, it's just unproductive, writing that code by myself would be faster.

@Rohith-Raju Rohith-Raju marked this pull request as draft February 10, 2023 01:33
@Rohith-Raju
Copy link
Contributor Author

Request struct needs special care: it contains transforms and matchers callbacks invoked during Expect() call. We > should NOT call them under a lock because otherwise it would be easy to write code with deadlocks (if you use the same > > request from this callbacks).

Yes, I'm sorry. I totally ignored this. Won't happen again.

@Rohith-Raju Rohith-Raju deleted the requestThreadSafety branch February 11, 2023 13:26
@Rohith-Raju
Copy link
Contributor Author

Deleted this as the changes are irrelevant.

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.

3 participants