Add a dio_test project and move request integration test #2077
Conversation
kuhnroyal
added
p: dio
kuhnroyal
force-pushed
the
chore/test-utils
branch
2 times, most recently
from
aafb558 to
807f2f3
Compare
|
@AlexV525 What do you think about this approach? There is also a bugfix for redirects in here. |
dio_tests/pubspec.yaml
Outdated
| publish_to: none | ||
|
|
||
| environment: | ||
| sdk: '>=3.0.0 <4.0.0' |
There was a problem hiding this comment.
If we're doing this, we should not be able to run this with our minimum SDK support.
There was a problem hiding this comment.
Yea, this is due to the HTTP2 requiring this.
I don't see a way to create a utils package that we can use in all the adapters. We would have a cyclic dependency on dio->util->dio
There was a problem hiding this comment.
I mean we cannot run the dio/test/request_integration_test.dart anymore on 2.15 after this, correct?
There was a problem hiding this comment.
Yes, I don't see a way to run the same test suite on lower SDK if one of our adapters requires 3.0.0.
Unless we can maybe run the same tests with different pubspec.yaml files?
There was a problem hiding this comment.
It actually works with a shared util package that we can import as dev dependency.
This way we can run tests on all supported SDK versions.
There was a problem hiding this comment.
Update this a bit but we now have a shared dio_test package that is imported as dev dependency into dio and http2_adapter. So the tests can be run with all Dart SDK targets.
Can we extract the fix to another request first? |
Done. See #2101 |
kuhnroyal
force-pushed
the
chore/test-utils
branch
7 times, most recently
from
2040a63 to
f8b1d08
Compare
Fixes a redirect bug discovered during #2077 where the adapter would fail to properly redirect if the location header contains a relative URL. <!-- Write down your pull request descriptions. --> ### New Pull Request Checklist - [x] I have read the [Documentation](https://pub.dev/documentation/dio/latest/) - [x] I have searched for a similar pull request in the [project](https://github.com/cfug/dio/pulls) and found none - [x] I have updated this branch with the latest `main` branch to avoid conflicts (via merge from master or rebase) - [x] I have added the required tests to prove the fix/feature I'm adding - [x] I have updated the documentation (if necessary) - [x] I have run the tests without failures - [x] I have updated the `CHANGELOG.md` in the corresponding package ### Additional context and info (if any) <!-- Provide more context and info about the PR. -->
* run tests for all adapters * fix redirect bug for `Http2Adapter`
kuhnroyal
force-pushed
the
chore/test-utils
branch
from
f8b1d08 to
21b5f75
Compare
kuhnroyal
force-pushed
the
chore/test-utils
branch
from
9177310 to
6e812bd
Compare
kuhnroyal
force-pushed
the
chore/test-utils
branch
from
3c77ae9 to
2c3abd2
Compare
kuhnroyal
force-pushed
the
chore/test-utils
branch
from
58c6cd4 to
f37c5e2
Compare
|
I moved some tests in to the test package but I think we should move the rest in separate PRs. So I think this is good to go if we want to take that route. |
This was supposed to be just an attempt but while moving the integration test, I found a bug since one of the tests failed for the
Http2Adapter.So I think this is a good approach, I will move more tests in future PRs.
Http2AdapterNew Pull Request Checklist
mainbranch to avoid conflicts (via merge from master or rebase)CHANGELOG.mdin the corresponding packageAdditional context and info (if any)