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

Make integration test fully async #426

Merged
merged 17 commits into from
Nov 21, 2018

Conversation

slang25
Copy link
Member

@slang25 slang25 commented Nov 10, 2018

Attempt to kill almost all blocking code, this removes all the occurences in the test projects. There are only 2 places left now:

  • Fluent API, this we are well aware of
  • Thottled.cs, this one is tricky and could be hit under contention when at throttling limits, I have an idea on how to fix this in a non-breaking way. For another PR.

Something is going on with line-endings, I don't know if it's me, or if the code base needs all lines normalizing. I'll investigate.

@slang25 slang25 requested a review from a team as a code owner November 10, 2018 23:46
@martincostello
Copy link
Member

There’s been a few seemingly massive diffs in other PRs due to line endings too. I guess we’re in need of a .gitattributes fix to ensure we’re not at the mercy of people’s local settings?

@AnthonySteele
Copy link
Contributor

Hm, we have a .editorconfig, but sometimes that's not enough and a compatible .gitattributes file is also needed.

@slang25
Copy link
Member Author

slang25 commented Nov 11, 2018

Because we were hitting the fluent api code (that blocks) previously in the constructor, it was "harmless".

Now we are using async initialization, we deadlock the xunit SynchronizationContext.
There are lot's of hacky workarounds, I've opted here for disabling concurrent tests (which means xunit doesn't use it's special context). 🤠

@slang25 slang25 added this to the v7.0.0 milestone Nov 11, 2018
@martincostello
Copy link
Member

@slang25 I noticed you have an old fluent-refactor lying around here. Do you still need it?

@slang25
Copy link
Member Author

slang25 commented Nov 13, 2018

I'd like to keep it for reference, I'll move it into my fork and delete it from this repo

@slang25
Copy link
Member Author

slang25 commented Nov 14, 2018

Any progress on a JustBehave release?

@adammorr
Copy link
Contributor

@slang25 https://www.nuget.org/packages/JustBehave/2.0.0-beta-62 will be a thing once NuGet updates

@slang25
Copy link
Member Author

slang25 commented Nov 15, 2018

Thanks @adammorr !

@slang25 slang25 changed the title [WIP] Make integration test fully async Make integration test fully async Nov 20, 2018
@martincostello
Copy link
Member

I’ll review this tomorrow 👍🏻

@slang25
Copy link
Member Author

slang25 commented Nov 20, 2018

Thanks, it's pretty boring I'm afraid!

AnthonySteele
AnthonySteele previously approved these changes Nov 20, 2018
Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor points but otherwise, great job!

martincostello
martincostello previously approved these changes Nov 21, 2018
Copy link
Member

@martincostello martincostello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@slang25
Copy link
Member Author

slang25 commented Nov 21, 2018

I'll look at the failure over lunch

@slang25
Copy link
Member Author

slang25 commented Nov 21, 2018

I've put the tests back to running sequentially now, there are too many areas that require exclusivity. Ready now @martincostello

@martincostello
Copy link
Member

Go go gadget tests!

@martincostello martincostello merged commit 42f1bc2 into justeattakeaway:master Nov 21, 2018
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.

4 participants