Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[flutter_plugin_tools] Add Linux support to native-test #4294

Merged
merged 11 commits into from
Sep 1, 2021
Merged

[flutter_plugin_tools] Add Linux support to native-test #4294

merged 11 commits into from
Sep 1, 2021

Conversation

stuartmorgan-g
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g commented Sep 1, 2021 •
edited
Loading

  • Adds a minimal unit test to url_launcher_linux as a proof of concept. This uses almost exactly the same CMake structure as the Windows version that was added recently.
  • Adds Linux support for unit tests to native-test, sharing almost all of the existing Windows codepath.
  • Fixes the fact that it it was running the debug version of the unit tests, but build-examples only builds release. (On other platforms we run debug unit tests, but on those platforms the test command internally builds the requested unit tests, so the mismatch doesn't matter.)
  • Enables the new test in CI.

Also opportunistically fixes some documentation in native_test_command.dart that wasn't updated as more platform support was added.

Linux portion of flutter/flutter#82445

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

@google-cla google-cla bot added the cla: yes label Sep 1, 2021
@stuartmorgan-g
Copy link
Contributor Author

Hrm, I'll need to debug why it's not finding the tests when running in CI tomorrow; it worked for me locally.

@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Sep 1, 2021 •
edited
Loading

Hrm, I'll need to debug why it's not finding the tests when running in CI tomorrow; it worked for me locally.

This is fixed now; build-examples builds release, and native-test was only running debug, so the combination used by CI didn't actually work. PR description updated with the added change.

cbracken
cbracken reviewed Sep 1, 2021
View reviewed changes
packages/url_launcher/url_launcher_linux/linux/url_launcher_plugin_private.h Outdated
// Handles the canLaunch method call.
//
// Temporarily exposed for testing due to
// https://github.com/flutter/flutter/issues/88724
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit consider phrasing as a TODO to aid in tech-debt greppability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, done.

cbracken
cbracken reviewed Sep 1, 2021
View reviewed changes
script/tool/lib/src/native_test_command.dart
// same tests multiple times.
// Only run the release build of the unit tests, to avoid running the
// same tests multiple times. Release is used rather than debug since
// `build-examples` builds release versions.
Copy link
Member

@cbracken cbracken Sep 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was going to say perhaps we should be changing to generate a debug build of build-examples given that it's typically more convenient to debug debug builds. That said, in the end you want to verify the release build of the library code in the plugin, and people shouldn't be relying on #ifdef'ed code in their tests, so this seems fine.

If this ever gets problematic, we could add a buildmode flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that there's actually a more annoying problem, which is that the current behavior isn't going to be good for local use. Filed flutter/flutter#89303.

I'm going to do that as a follow-up since it applies to the already-landed Windows flow in the same way, but I'll do it shortly.

cbracken
cbracken approved these changes Sep 1, 2021
View reviewed changes
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@stuartmorgan-g stuartmorgan-g merged commit df75b01 into flutter:master Sep 1, 2021
@stuartmorgan-g stuartmorgan-g deleted the linux-unit-test branch September 1, 2021 18:18
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 1, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 1, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
2b8e9f5 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 1, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 1, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
4b9a7dc 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 2, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 2, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
f45911d 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 2, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 2, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
bab1763 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 7, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 7, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
d6611e2 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
878ff5a 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
2bf01ac 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
a09f9fc 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
2466315 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 8, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 8, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
f4cf502 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
1fa9265 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
1d3ff3d 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
14c75d6 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
a83c097 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
06158e2 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
4fe5873 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
154183b 
…ter/plugins#4294)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 9, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Sep 9, 2021
@engine-flutter-autoroll
df75b01 [flutter_plugin_tools] Add Linux support to native-test (flut…
dc4e0a8 
…ter/plugins#4294)
fotiDim pushed a commit to fotiDim/plugins that referenced this pull request Sep 13, 2021
@stuartmorgan-g @fotiDim
[flutter_plugin_tools] Add Linux support to native-test (flutter#4294)
0b5af41 
- Adds a minimal unit test to url_launcher_linux as a proof of concept. This uses almost exactly the same CMake structure as the Windows version that was added recently.
- Adds Linux support for unit tests to `native-test`, sharing almost all of the existing Windows codepath.
- Fixes the fact that it it was running the debug version of the unit tests, but `build-examples` only builds release. (On other platforms we run debug unit tests, but on those platforms the test command internally builds the requested unit tests, so the mismatch doesn't matter.)
- Enables the new test in CI.

Also opportunistically fixes some documentation in `native_test_command.dart` that wasn't updated as more platform support was added.

Linux portion of flutter/flutter#82445
amantoux pushed a commit to amantoux/plugins that referenced this pull request Sep 27, 2021
@stuartmorgan-g @amantoux
[flutter_plugin_tools] Add Linux support to native-test (flutter#4294)
622cd7f 
- Adds a minimal unit test to url_launcher_linux as a proof of concept. This uses almost exactly the same CMake structure as the Windows version that was added recently.
- Adds Linux support for unit tests to `native-test`, sharing almost all of the existing Windows codepath.
- Fixes the fact that it it was running the debug version of the unit tests, but `build-examples` only builds release. (On other platforms we run debug unit tests, but on those platforms the test command internally builds the requested unit tests, so the mismatch doesn't matter.)
- Enables the new test in CI.

Also opportunistically fixes some documentation in `native_test_command.dart` that wasn't updated as more platform support was added.

Linux portion of flutter/flutter#82445
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cbracken cbracken cbracken approved these changes

Assignees
No one assigned
Labels
cla: yes p: url_launcher platform-linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stuartmorgan-g @cbracken