[connectivity] Web version #2545
Conversation
|
There's a bunch of formatting BS going on, but worst of all is that I don't have tests. I want to experiment with |
|
Some tests now run: |
|
After the whole mocking I did, I just realized that these are proper unit tests (that probably don't need to run in a browser at all). I'm going to refactor my tests for them to be just normal unit tests, and remove unneeded dependencies. |
|
Nope: |
packages/connectivity/connectivity_web/ios/Classes/SwiftConnectivityWebPlugin.swift
Outdated
Show resolved
Hide resolved
|
Just curious: does the use of NetworkInformation API mean it doesn't do anything in Firefox and Safari, since these browsers do not support the NetworkInformation API by default? |
|
@woutervanwijk correct, in other browsers it reports "offline" by default, but it could be changed to any other result. I'm thinking of returning 'null' for the cases where the connectivity cannot be determined. |
| flutter run -d chrome | ||
| ``` | ||
|
|
||
| Contributions to this package are welcome. Read the [Contributing to Flutter Plugins](https://github.com/flutter/plugins/blob/master/CONTRIBUTING.md) guide to get started. |
There was a problem hiding this comment.
Since we already wrote the test using the e2e package. I wonder if we can also run them by using the driver?
There was a problem hiding this comment.
Yes, I want to do this. The easier it is to run the tests in the package, the better!
| expect(plugin.checkConnectivity(), completion(equals(expected))); | ||
| } | ||
|
|
||
| group('in Chrome', () { |
There was a problem hiding this comment.
suggestion: we can add a browser based skip, if we want to show if something runs on chrome but does not run in any other browser. In CI we can run it against two different drivers? (we can also do it later when we are adding this to CI)
There was a problem hiding this comment.
Is this done with the @TestOn('chrome') annotation, or do I need to split this test in different files per-browser?
There was a problem hiding this comment.
There was a problem hiding this comment.
Interesting! Right now I'm mocking the behavior of other browsers, but that can get obsolete. It'd be great to be able to run these tests in multiple browsers and get the actual API doing its thing.
There was a problem hiding this comment.
Yes we can run it on multiple browsers when you are adding them to CI.
We can change the argument for browser-name, and choose another browsers when we are starting a drive test.
https://github.com/flutter/flutter/blob/master/.cirrus.yml#L288
|
Bringing this back to life. Rebased with the latest master, and returned |
This reverts commit 43494c6.
```
Checking that connectivity_web can be published.
Suggestions:
* line 1, column 1 of test/lib/main.dart: This package does not have connectivity_web_example in the `dependencies` section of `pubspec.yaml`.
╷
1 │ import 'package:connectivity_web_example/src/connectivity_mocks.dart';
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
╵
* line 2, column 1 of test/lib/main.dart: This package does not have e2e in the `dependencies` section of `pubspec.yaml`.
╷
2 │ import 'package:e2e/e2e.dart';
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
╵
* line 3, column 1 of test/lib/main.dart: This package does not have flutter_test in the `dependencies` section of `pubspec.yaml`.
╷
3 │ import 'package:flutter_test/flutter_test.dart';
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
╵
Package has 3 warnings.
```
Surprisingly, the script doesn't detect that "test" is a package of its own, and it requires the connectivity_web package to dev-depend on things that are only used by "test"...
Make private members final.
|
I'm going to land this into an experimental branch in my fork of flutter/plugins. If more contributions come (for more browsers, etc...) let's push it to the official repo. But for now, IMO, this is not fully flutter-worthy. |
|
I'm publishing this package from this branch, instead of merging into flutter/plugins:master, until it is mature enough. |
|
Tagged and published experimental_connectivity_web. |
Description
This is a version of the connectivity plugin for the web platform, using the experimental NetworkInformation API with a (mostly) auto-generated JS Interop facade.
Related Issues
Fixes flutter/flutter#46735
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?