-
-
Notifications
You must be signed in to change notification settings - Fork 183
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
aiomoto #766
aiomoto #766
Conversation
162f3e1
to
1b64be9
Compare
Pushed up a revision to add a
Notes
|
5d0b1ff
to
7a0abc2
Compare
Codecov Report
@@ Coverage Diff @@
## master #766 +/- ##
===========================================
- Coverage 90.83% 56.71% -34.12%
===========================================
Files 12 16 +4
Lines 611 878 +267
===========================================
- Hits 555 498 -57
- Misses 56 380 +324
Continue to review full report at Codecov.
|
I can't reproduce the travis test failures locally, it seems to be some problem with the virtualenv not getting all the project packages installed or found for the tests. Unfortunately, it works for me. |
wow awesome, will take a look if someone doesn't get to this before me |
btw the idea for the tests, in regard to buckets is that each test gets a unique bucket, see https://github.com/aio-libs/aiobotocore/blob/master/tests/conftest.py#L30 |
|
Actually ignore above rec for separate process per test. pytest-xdist already separates each test into a separate process |
This looks great 😄 Might have to loot some of it for aioboto3 once its merged 😉 |
This pull request introduces 1 alert when merging da1c4d6 into fcf7cd4 - view on LGTM.com new alerts:
|
Need to consider options to parametrize the server and client fixtures to address the code-cov problem. |
This pull request introduces 2 alerts when merging 1c24619 into 904332b - view on LGTM.com new alerts:
|
The PR was rebased on master and seems to pass unit tests OK.
It's in a state that might be "good enough to go", although there might be additional feature requests that subsequent PRs can address. I have enough code to continue with the "real" work that needs some of this and my focus needs to shift to that. Hope this helps a bit. I did not add any documentation about this, because it could be premature to advertise it. |
Hi @dazza-codes, do you know if there are any plans to merge this?, I'm able to reproduce the issue Thanks in advance. |
My focus on this has moved to https://gitlab.com/dazza-codes/aio-aws and I've lost track of where this PR is at in the context of this project. I doubt it's in the right form exactly to be merged for this project. I don't know if I have time and/or enough feedback to get it into the right shape for this repo. I understand there are some other branches with various refactoring that might conflict with this, so it's probably not worth the time right now to resolve anything until there's a new release. Maybe the project maintainers can just cherrypick patches from this into the main line. I know it took a lot of time and reading to understand the problem and put together this test suite, so let's see what happens as the project refactoring evolves. At least this PR is visible as one possible solution to the test problems. |
Thanks @dazza-codes, I forked and merged your PR and it didn't work for DynamoDB, need to change the approach to test. |
aiobotocore/aiomoto/aws_fixtures.py
Outdated
|
||
@pytest.fixture | ||
def aws_port(): | ||
return os.getenv("AWS_PORT", AWS_PORT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should use a random port or else you can't run tests in parallel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a new commit to try to use a free port, although I'm not sure the patch will be used in moto.
sorry for taking so long to review this, been super busy |
I haven't been able to keep up with this and I'm a bit blocked on it since the move to |
nice, we were on pipenv, then got frustrated due to slowness/inability to resolve certain scenarios so moved to poetry, then found another blocking bug on it so switched back :). I initially tried the new pipenv release but it was completely busted so waiting a bit more to try the latest pipenv again |
hello, @dazza-codes I too am reproducing this error 'AWSResponse' object has no attribute 'raw_headers' when testing with moto. I tried using the patched code solution suggested in https://github.com/aio-libs/aiobotocore/issues/755#issuecomment-586117397 but it doesn't seem to affect the issue. Maybe I missed it in this thread, but is there a better way to deal with this error? |
This pull request has been marked as stale because it has been inactive for more than 60 days. Please update this pull request or it will be automatically closed in 7 days. |
btw new version of moto does not require an endpoint per service, all services are serviced by the same endpoint so will not be needed. I started working on that here: master...thehesiod/aio-checksum-streaming-moto-bump bunch of tests to fix |
Description of Change
Fix #583
Fix #665
Fix #753
Fix #755
Fix getmoto/moto#2706
This most likely replaces #759 and that PR should not be merged.
Notes
MotoService
does not provide a clean backend across tests, by default, so the server fixtures in this PR add and use aMotoService.reset
option to clear the backends