HomeX-It / snowplow-flutter-tracker

Snowplow event tracker for Flutter. Add analytics to your Flutter apps and games http://snowplowanalytics.com
MIT License
5 stars 7 forks source link

Adds SnowplowFlutterTracker.close() tests #29

Closed ohitsdaniel closed 3 years ago

ohitsdaniel commented 3 years ago

A bit too fast on the merge button. πŸ˜„

I added some more tests for the .close() function as well.

Do you think, we should throw, when the library user tries to close a tracker that hasn't been initialised?

codecov[bot] commented 3 years ago

Codecov Report

Merging #29 (c0afea3) into main (96da2cf) will increase coverage by 0.57%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #29      +/-   ##
==========================================
+ Coverage   13.71%   14.28%   +0.57%     
==========================================
  Files          28       28              
  Lines         649      651       +2     
==========================================
+ Hits           89       93       +4     
+ Misses        560      558       -2     
Impacted Files Coverage Ξ”
lib/src/snowplow_flutter_tracker.dart 27.77% <100.00%> (+10.13%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data Powered by Codecov. Last update 96da2cf...c0afea3. Read the comment docs.

ohitsdaniel commented 3 years ago

We could also remove the throw from the init and just ignore the call, when it was already initialized, just like in close now. :)

MisterJimson commented 3 years ago

@ohitsdaniel If I call init again as a user, I would expect any previous values to be replaced by the new ones I am providing in the init call. If we can do that, then I would allow multiple inits.

If we cannot replace the init call at the native layer (so calling init a second time would basically ignore) we should throw so the user knows the init is not going to work/do anything.

ohitsdaniel commented 3 years ago

Snowplow currently does not allow more than once instance to be initialized on the native layer. This will change with Release 2.0 and we have an issue to allow multiple tracker instances as well.

For now, I think the current throwing behavior is fine. Let's merge it then. :)