search

bugsnag / bugsnag-cocoa-performance

Monitor the start-up, screen loading and network requests of your iOS app and see the results in your BugSnag dashboard.
https://docs.bugsnag.com/performance/integration-guides/ios
MIT License
11 stars 8 forks source link

Ensure e2e test fixture clears data at the beginning of each scenario #258

Closed Cawllec closed 2 months ago

Cawllec commented 2 months ago

Goal

Since the performance library begins recording data as soon as the app starts we may receive anomalous data which can cause our tests to flake. This change ensures that the test fixture can recognise the first command received during a test scenario, and reset the application data at that point.

This would mean we can remove clearPersistentData calls from individual scenarios, but I've not implemented that at this point.

Note: When the clearing occurs we log several messages in the vein of:

BUG IN CLIENT OF libsqlite3.dylib: database integrity compromised by API violation: vnode unlinked while in use

This may be undesirable, but doesn't appear to effect the operation of the tests themselves. However, if there is a clearer method to clear the persistent data that wouldn't result in these error logs, that may be worth implementing.

github-actions[bot] commented 2 months ago

BugsnagPerformance.framework binary size did not change - 445,632 bytes

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [ = ]       0  [ = ]       0    TOTAL

Generated by :no_entry_sign: Danger

kstenerud commented 2 months ago

I feel nervous about driving behaviour from the size of the command queue (it would leave a landmine attached to the command queue implementation that's non-obvious). Could we instead add a "clear_data_before_running" field to all commands? Then we have more precise control and the intent is very clear.

Cawllec commented 2 months ago

On a second review of this, is it really necessary? We've got a clearPersistentData call in the scenario, explicitly called at the beginning of every test scenario. We could expand that to clear the caches as well as the UserDefaults (basically update it to act like the clearPersistentData function added here.