Use before_all block to setup requests/cache_spec data

[mjankowski]

This spec is looping through various collections of endpoints and asserting different caching properties.

Previously, the before block here was running before every example, and thus creating/dropping the full set of factories in every example-wrapping transaction.

The change here is to use before_all which wraps the example group in a big transaction and creates the factories (which, in this entire spec, are very much like fixture data). The individual transactions around each example from rspec are still in place, so for the few examples which do modify any of the records, that will not pollute other specs.

Some before/after summary to show factory creation reduction:

BEFORE:

Finished in 23.1 seconds (files took 2.78 seconds to load)
253 examples, 0 failures

[TEST PROF INFO] Factories usage

 Total: 1611
 Total top-level: 1611
 Total time: 00:12.677 (out of 00:23.892)
 Total uniq factories: 6

   total   top-level     total time      time per call      top-level time               name

     506         506        2.6162s            0.0052s             2.6162s             status
     264         264        3.4650s            0.0131s             3.4650s            account
     253         253        3.5710s            0.0141s             3.5710s             invite
     253         253        0.4301s            0.0017s             0.4301s               poll
     253         253        2.4347s            0.0096s             2.4347s               user
      82          82        0.1603s            0.0020s             0.1603s accessible_access_token

AFTER:

Finished in 9.54 seconds (files took 2.72 seconds to load)
253 examples, 0 failures

[TEST PROF INFO] Factories usage

 Total: 18
 Total top-level: 18
 Total time: 00:00.860 (out of 00:10.325)
 Total uniq factories: 6

   total   top-level     total time      time per call      top-level time               name

      12          12        0.6972s            0.0581s             0.6972s            account
       2           2        0.0997s            0.0499s             0.0997s             status
       1           1        0.0093s            0.0093s             0.0093s               user
       1           1        0.0232s            0.0232s             0.0232s             invite
       1           1        0.0134s            0.0134s             0.0134s               poll
       1           1        0.0175s            0.0175s             0.0175s accessible_access_token

So, we are able to reduce:

• Total factory creation from ~1600 DB records to just under 20
• Percentage of time spent on factories from ~50% to under 10%
• Overall spec file run time by more than 50%

[mjankowski]

Failure here seems like CI issue, not code issue...

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.10%. Comparing base (86500e3) to head (95d8b31).
Report is 241 commits behind head on main.

❗ Current head 95d8b31 differs from pull request most recent head 4c90de6. Consider uploading reports for the commit 4c90de6 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #29437      +/-   ##
==========================================
+ Coverage   85.01%   85.10%   +0.08%     
==========================================
  Files        1059     1062       +3     
  Lines       28277    28352      +75     
  Branches     4538     4548      +10     
==========================================
+ Hits        24040    24129      +89     
+ Misses       3074     3061      -13     
+ Partials     1163     1162       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ClearlyClaire]

I am not too comfortable with using something from test-prof for this, but it seems reasonable and a noticeable performance increase.

[mjankowski]

I am not too comfortable with using something from test-prof for this, but it seems reasonable and a noticeable performance increase.

I basically agree -- and I think if we didn't already have the gem included for perf stuff I wouldn't have pulled it in just for this method. There's a version of this where we can use before(:all) and after(:all) hooks together with DatabaseCleaner to get a similar result ... do you want to see a variation of this using that approach and choose between them? Or are we good with this?

[ClearlyClaire]

I am not too comfortable with using something from test-prof for this, but it seems reasonable and a noticeable performance increase.

I basically agree -- and I think if we didn't already have the gem included for perf stuff I wouldn't have pulled it in just for this method. There's a version of this where we can use before(:all) and after(:all) hooks together with DatabaseCleaner to get a similar result ... do you want to see a variation of this using that approach and choose between them? Or are we good with this?

I think I still prefer this to before(:all) + after(:all) with DatabaseCleaner, the latter can be pretty surprising and has been the cause of a number of issues when writing system specs.