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

chore: Improve testing #1873

Merged
merged 16 commits into from Aug 2, 2022
Merged

chore: Improve testing #1873

merged 16 commits into from Aug 2, 2022

Conversation

dnys1
Copy link
Contributor

@dnys1 dnys1 commented Jul 12, 2022

Use build_test package to test both DDC and dart2js in CI which will help catch discrepancies earlier in our dev cycle. This requires separating out the tests into a new, private package so that build_runner can correctly discern the dependency graph.

@dnys1 dnys1 requested a review from a team as a code owner July 12, 2022 15:51
@dnys1 dnys1 marked this pull request as draft July 12, 2022 15:51
@dnys1 dnys1 force-pushed the chore/ci/test-commands branch 2 times, most recently from c42ac3a to 34813d9 Compare July 12, 2022 15:59
@dnys1 dnys1 marked this pull request as ready for review July 12, 2022 16:08
@dnys1 dnys1 marked this pull request as draft July 12, 2022 18:01
@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2022

Codecov Report

Merging #1873 (50f8dc1) into next (d86d8d9) will decrease coverage by 0.51%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             next    #1873      +/-   ##
==========================================
- Coverage   44.04%   43.53%   -0.52%     
==========================================
  Files         122      118       -4     
  Lines        7529     7427     -102     
==========================================
- Hits         3316     3233      -83     
+ Misses       4213     4194      -19     
Flag Coverage Δ
android-unit-tests ∅ <ø> (∅)
flutter-unit-tests 26.74% <ø> (ø)
ios-unit-tests 89.48% <ø> (+0.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...plify_authenticator/lib/amplify_authenticator.dart 0.00% <ø> (ø)
...plify_flutter/example/ios/Runner/AppDelegate.swift
...e/ios/unit_tests/MockAnalyticsCategoryPlugin.swift
.../ios/unit_tests/amplify_flutter_exampleTests.swift
...ter/example/ios/unit_tests/AtomicResultTests.swift

@dnys1 dnys1 force-pushed the chore/ci/test-commands branch 3 times, most recently from 383da6c to 4b23feb Compare July 12, 2022 23:45
@dnys1 dnys1 marked this pull request as ready for review July 13, 2022 02:01
@dnys1 dnys1 changed the title chore(auth): Improve testing chore: Improve testing Jul 13, 2022
@dnys1 dnys1 force-pushed the chore/ci/test-commands branch 2 times, most recently from 1c14e5c to a041eef Compare July 19, 2022 02:30
```sh
$ dart test
$ dart run build_runner test --delete-conflicting-outputs -- -p chrome,firefox
$ dart run build_runner test --release --delete-conflicting-outputs -- -p chrome,firefox
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this work with Edge, like Flutter tests? If so, might be worth noting somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah, I forget about edge 😂 I'll see if we can get it running on windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it's not supported yet, unfortunately: dart-lang/test#1142. The Flutter team jumped through some hoops to get it to work (flutter/engine#15641) and I'm not sure if that effort is worth it right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, sounds good

Copy link
Contributor

@haverchuck haverchuck left a comment

Choose a reason for hiding this comment

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

Left one nit, but otherwise looks great.

@dnys1 dnys1 force-pushed the chore/ci/test-commands branch 2 times, most recently from 615ebc2 to 1734bcc Compare July 19, 2022 21:26
- linux
# TODO(dnys1): Secure storage will not work in DDC right now due to ongoing staticInterop changes.
# Since the interfaces are only ever called via workers (dart2js), this is not a problem but should be monitored.
# Issue: https://github.com/dart-lang/sdk/issues/49301
Copy link
Contributor

Choose a reason for hiding this comment

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

This issue is closed. Is this something that has been resolved?

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'll double check - may only be fixed in dev

Dillon Nys added 10 commits August 2, 2022 09:32
Use `build_test` package to test both DDC and dart2js in CI which will help catch discrepancies earlier in our dev cycle.

commit-id:94bb9eb4
Refactors worker_bee tests to allow for DDC/dart2js testing

commit-id:87173490
Moves tests to the `_test` package so that they can be run uses `build_runner` which can test DDC/dart2js.

commit-id:6130a074
commit-id:aba66832
commit-id:ed4cd5a5
This global helps disambiguate between the multiple combinations of debug, release, and test modes without additional flags needing to be passed to test commands. Potentially, `zDebugMode` could be replaced with this since it's technically more accurate for non-Flutter applications, but since it's non-const we'd lose out on tree-shaking and other compile-time benefits.

commit-id:80dc3075
Fixes multiple tests in the new setup by correctly setting build flags and overriding timeouts for browsers where tests with workers run really slow for some reason (not indicative of worker performance).

commit-id:30175071
Correctly sets build flags for examples so that `zDebugMode` and `zReleaseMode` function correctly

commit-id:605007b2
commit-id:124763b6
Dillon Nys added 5 commits August 2, 2022 09:36
commit-id:c2b9908c
Add `--enable-asserts` for dart2js release code

commit-id:c977cea4
It's failing in CI currently and I cannot reproduce locally

commit-id:ffcb537d
commit-id:8b80d358
@dnys1 dnys1 merged commit f64e4c1 into aws-amplify:next Aug 2, 2022
@dnys1 dnys1 deleted the chore/ci/test-commands branch August 2, 2022 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants