Migrate state_notifier to null-safety

[gaetschwartz]

Migrated state_notifier to null-safety, tests are not migrated yet because mockito hasn't migrated yet. They all pass though !

[codecov[bot]]

Codecov Report

Merging #42 (6711f1b) into master (a138997) will increase coverage by 5.12%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
+ Coverage   89.91%   95.03%   +5.12%     
==========================================
  Files           4        5       +1     
  Lines         228      463     +235     
==========================================
+ Hits          205      440     +235     
  Misses         23       23              

Impacted Files	Coverage Δ
...kages/state_notifier/test/state_notifier_test.dart	100.00% <ø> (ø)
packages/state_notifier/lib/state_notifier.dart	97.59% <100.00%> (-0.22%)	:arrow_down:

[gaetschwartz]

CI is failing because it's only on Beta and Dev hence we'd need to publish it as a prerelease. I'm unsure about how to version this so I put 0.7.0-nullsafety as it seems to be what others do ?

[rrousselGit]

Indeed, we'll have to do that.

[gaetschwartz]

Coverage seems to fail on Beta for some reason. I don't know test_coverage enough to say more

[rrousselGit]

The tests are failing I think (not as in we have a bug, but rather because of NNBD). See the CI on master – that's probably what is happening here.

[gaetschwartz]

It is test_coverage that is failing, the tests pass. Why not use dart test --coverage coverage instead ? Saves the coverage file as a .json in coverage/test/.

[rrousselGit]

Why not use dart test --coverage coverage instead ? Saves the coverage file as a .json in coverage/test/.

Codecov requires an lcov file, which is not generated by dart test --coverage. This scripts generates one

We can remove test_coverage, but we need to generate an lcov file.

[gaetschwartz]

flutter test --coverage does though. Strange that dart doesn't. Maybe we can use flutter for now ?

[rrousselGit]

We can't. flutter test does not work on Dart-only packages, and state_notifier is one

[gaetschwartz]

It does on my machine at least, just have to add

  flutter_test:
    sdk: flutter

to the pubspec.yaml and it does, unless you specifically want to use dart test it might be an alternative. But I don't really understand why is test_coverage failing. It's probably not compatible with NNBD I guess?

[rrousselGit]

We can't add flutter_test as dependency. The package must not depend on flutter

[gaetschwartz]

Hum okay so apparently dart test and dart .\test\state_notifier_test.dart don't behave the same, dart test works but the latter fails, will investigate more and try to fix these.

[gaetschwartz]

Indeed test_coverage uses dart .\test\state_notifier_test.dart which also fails on master. I don't really know what that implies, are the tests actually failing and dart test is inaccurate or the opposite and using dart .\test\state_notifier_test.dart doesn't make any sense ?

[rrousselGit]

You need to make sure that you passed the --enable-asserts flag, otherwise, the code behaves differently

[gaetschwartz]

Alright so the tests indeed pass, I think the issue is that test_coverage generates a file that doesn't start with

[gaetschwartz]

Alright this should fix CI, instead of using test_coverage, we use coverage. The package translates the .json coverage file generated by dart test --coverage to to a lcov one.

Edit : There we go it's fixed !

[rrousselGit]

Thanks again! Using coverage sounds interesting. I'll play around with it too, to see if that works correctly