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

Add mock httpClient for image tests #17

Merged
merged 4 commits into from Dec 8, 2017

Conversation

Projects
None yet
2 participants
@xqwzts
Copy link
Contributor

xqwzts commented Dec 8, 2017

Creates a mock http.Client that always returns StatusCode 200 and an image [taken from https://github.com/flutter/flutter/blob/master/dev/manual_tests/test/mock_image_http.dart] and uses it as the httpClient for image tests.

More info here: flutter/flutter#13433

@xqwzts xqwzts referenced this pull request Dec 8, 2017

Merged

Inline images #14

@DaveShuckerow DaveShuckerow self-requested a review Dec 8, 2017

@DaveShuckerow
Copy link

DaveShuckerow left a comment

Two minor changes and you're good to go.

});
};

const List<int> _kTransparentImage = const <int>[

This comment has been minimized.

Copy link
@DaveShuckerow

DaveShuckerow Dec 8, 2017

The 'k' isn't necessary for constant names. Go ahead and drop that: const _transparentImage

This comment has been minimized.

Copy link
@xqwzts

xqwzts Dec 8, 2017

Author Contributor

Done

@@ -165,6 +168,8 @@ void main() {
});

testWidgets('Image links', (WidgetTester tester) async {
createHttpClient = createMockImageHttpClient;

This comment has been minimized.

Copy link
@DaveShuckerow

DaveShuckerow Dec 8, 2017

Suggestion:

Move these into a group and prepare the image client in setUpAll():

group('Images', () {
  setUpAll(() {
    createHttpClient = ...;
  });
  testWidgets('should work with a link', ...);
  testWidgets('should show properly next to text', ...);
});

The style for naming test groups is for the group name to make a complete sentence when joined with each test's name.

This comment has been minimized.

Copy link
@xqwzts

xqwzts Dec 8, 2017

Author Contributor

Done.

xqwzts added some commits Dec 8, 2017

@xqwzts

This comment has been minimized.

Copy link
Contributor Author

xqwzts commented Dec 8, 2017

Updated.

I also changed the test names for the links group to match the sentence style. [Wasn't sure if that would need its own PR]

@DaveShuckerow
Copy link

DaveShuckerow left a comment

Looks good to me.

@DaveShuckerow DaveShuckerow merged commit 5ed7502 into flutter:master Dec 8, 2017

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@xqwzts xqwzts deleted the xqwzts:mock-image-http-client branch Dec 8, 2017

trieutritam pushed a commit to trieutritam/flutter_markdown that referenced this pull request Sep 26, 2018

Add mock httpClient for image tests (flutter#17)
* Add mock httpClient for image tests

* Rename const _transparentImage

* Add group for images tests

* Change link test names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.