Skip to content

Conversation

@Guardiola31337
Copy link
Contributor

Description

Fixes flaky telemetry tests resetting the state in MapboxNavigationTelemetry#initialize

Regression from #3540

  • I have added any issue links
  • I have added all related labels (bug, feature, new API(s), SEMVER, etc.)
  • I have added the appropriate milestone and project boards

Goal

Fix flaky telemetry tests @Ignored in eb7dce6

Implementation

MapboxNavigationTelemetry is an object (singleton) :trollface: and originalRoute was retained across tests violating the Independent principle from F.I.R.S.T.. Tests executed before MapboxNavigationTelemetryTest could leave MapboxNavigationTelemetry in an undesired state which caused the flakiness / exception

depart_events_are_different_on_external_route
sessionState=ACTIVE_GUIDANCE
originalRoute=DirectionsRoute(#793)
needHandleReroute=true
routeProgress=null

In this particular case originalRoute was retained from a previous test causing depart_events_are_different_on_external_route to fail as routeProgress was null / not initialized when onRoutesChanged > handleReroute were executed.

We need to make sure that the state is reset appropriately.

Testing

  • I have tested locally (including SNAPSHOT upstream dependencies if needed) through testapp/demo app and run all activities to avoid regressions
  • I have tested via a test drive, or a simulation/mock location app
  • New and existing unit tests pass locally with my changes

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have updated the CHANGELOG including this PR
  • We might need to update / push api/current.txt files after running $> make core-update-api (Core) / $> make ui-update-api (UI) if there are changes / errors we're 🆗 with (e.g. AddedMethod changes are marked as errors but don't break SemVer) 🚀 If there are SemVer breaking changes add the SEMVER label. See Metalava docs

@Guardiola31337 Guardiola31337 added bug Defect to be fixed. tests labels Oct 28, 2020
@Guardiola31337 Guardiola31337 self-assigned this Oct 28, 2020
Comment on lines 133 to 134
reset()
needHandleReroute = false
needStartSession = false
sessionState = IDLE
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling reset at initialize should be enough to fix the flakiness but was wondering if we should reset the other state too (needHandleReroute, needStartSession and sessionState). Another thing that was wondering was if we should add needHandleReroute, needStartSession and sessionState as part of the reset which is also called from sessionStop 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should reset them all

Comment on lines 133 to 134
reset()
needHandleReroute = false
needStartSession = false
sessionState = IDLE
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should reset them all

@Guardiola31337 Guardiola31337 force-pushed the pg-fix-flaky-telem-tests branch from 6101b0b to 3b8e647 Compare October 29, 2020 13:43
@Guardiola31337 Guardiola31337 merged commit 0507408 into master Oct 29, 2020
@Guardiola31337 Guardiola31337 deleted the pg-fix-flaky-telem-tests branch October 29, 2020 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Defect to be fixed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants