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

Migrate tests #521

Merged
merged 7 commits into from
Mar 6, 2019
Merged

Migrate tests #521

merged 7 commits into from
Mar 6, 2019

Conversation

martincostello
Copy link
Member

As part of #380 (and eventually #471), this PR migrates some of the "old style" integration tests to the "newer way" of doing our integration tests.

These tests still use the queue classes directly in an integration-y way, but after this is merged I'll migrate them to setup via the fluent API so they're less "low-level".

The only "old" integration tests left now are the multi-region ones, which need a real AWS account to refactor them against as the emulator doesn't really support multi-region.

Change integration test to a unit test because it was using mocks to simulate AWS behaviour, so was more of a unit test.
Delete integration test that is doing low-level asserts to verify call counts. This should be rewritten as a unit test at a later point if it's really important, which I don't think it is.
Migrate integration test for error queue away from XAsyncBehaviourTest.
Migrate AwsTools integration tests away from XAsyncBehaviourTest. Next step will be to refactor them to go via the fluent interface, rather than with the direct objects.
Move attribute to root of project.
Move the "global" setup as it is now only used for multi-region tests.
@martincostello martincostello added this to the v7.0.0 milestone Feb 24, 2019
@martincostello martincostello requested a review from a team as a code owner February 24, 2019 17:38
@codecov
Copy link

codecov bot commented Feb 24, 2019

Codecov Report

Merging #521 into master will increase coverage by 0.36%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #521      +/-   ##
==========================================
+ Coverage    37.5%   37.86%   +0.36%     
==========================================
  Files          78       78              
  Lines        2608     2609       +1     
  Branches      453      453              
==========================================
+ Hits          978      988      +10     
+ Misses       1492     1481      -11     
- Partials      138      140       +2
Impacted Files Coverage Δ
...tSaying/AwsTools/MessageHandling/SnsTopicByName.cs 58.33% <0%> (+20.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74c2197...39b9235. Read the comment docs.

@AnthonySteele
Copy link
Contributor

Hard to review, since there are a lots of files and they don't match up - name, folder and contents have all changed so git can't tell which new one matches which old one.

@martincostello
Copy link
Member Author

All I can suggest is review one commit at a time - they're a rewrite, so they won't align, and the styles are fundamentally different anyway.

Alternatively, I can cherry pick each commit into a different PR each, but that's basically the same as looking at the diffs in each commit, except split over 6 PRs instead of one.

Update the name of a test to reduce confusion.
@martincostello martincostello requested a review from adammorr March 6, 2019 09:25
@martincostello martincostello merged commit 494afd7 into justeattakeaway:master Mar 6, 2019
@martincostello martincostello deleted the Migrate-Test branch March 6, 2019 15:00
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.

3 participants