🎨 [RUM-262] Move view metrics in dedicated files

[amortemousque]

Motivation

Isolate each view metrics and initial view timings in a dedicated file for easier maintenance and discoverability

Changes

before	after

Testing

• Local
• Staging
• Unit
• End to end

---

I have gone over the contributing documentation.

[bcaudan]

Should we do something with initialViewTimings as well?

[BenoitZugmeyer]

It seems you replaced the tests from this file with the test suite that was in trackViewMetrics.spec.ts. So, this spec file is not about trackViewEventCounts anymore: it's a higher level test suite that actually test the behavior of trackViewMetrics (actually, trackViews, since trackViewMetrics isn't used directly either).

I don't think that's a good idea. Spec files should be about testing the corresponding module, not a higher level module, else we won't have unit tests anymore and only integration tests.

I understand that it was already the case in the previous trackViewMetrics.spec.ts where trackViews was used (via setupViewTest) instead of trackViewMetrics, which isn't great, but I still think this PR shouldn't remove real unit tests.

[amortemousque]

Yes, the problem you describe is true for all the new viewMetrics/* modules.
While I agree on the fact that spec files should be about testing the corresponding module in this case it would mean rewriting lots of tests. This PR was more about readability and discoverability, so I'm wondering if revisiting these tests will bring much value for now. Wdyt?

Also about trackViewEventCounts original tests, to me it has a strange level of testing because it's mainly a wrapper around trackEventCounts which already has unit tests. So in that sens it could already be considered has integration tests which is why I merged it with the tests in trackViewMetrics.spec.ts

[BenoitZugmeyer]

Yes, the problem you describe is true for all the new viewMetrics/* modules. While I agree on the fact that spec files should be about testing the corresponding module in this case it would mean rewriting lots of tests. This PR was more about readability and discoverability, so I'm wondering if revisiting these tests will bring much value for now. Wdyt?

I agree, this is not the place or the time to change the tests :) my concern was more about removed tests

Also about trackViewEventCounts original tests, to me it has a strange level of testing because it's mainly a wrapper around trackEventCounts which already has unit tests. So in that sens it could already be considered has integration tests which is why I merged it with the tests in trackViewMetrics.spec.ts

I don't agree,trackViewEventCounts tests were specifically testing trackViewEventCounts logic (increment when receiving an event related to the view, don't increment when it's not related to the view, delayed stop)

[codecov-commenter]

Codecov Report

Merging #2386 (1a4eb40) into main (323d407) will decrease coverage by 0.02%.
Report is 3 commits behind head on main.
The diff coverage is 98.99%.

@@            Coverage Diff             @@
##             main    #2386      +/-   ##
==========================================
- Coverage   93.91%   93.89%   -0.02%     
==========================================
  Files         212      219       +7     
  Lines        6312     6311       -1     
  Branches     1405     1405              
==========================================
- Hits         5928     5926       -2     
- Misses        384      385       +1     

Files Changed	Coverage Δ
.../domain/rumEventsCollection/view/viewCollection.ts	100.00% <ø> (ø)
...ction/view/viewMetrics/interactionCountPolyfill.ts	47.05% <ø> (ø)
...ntsCollection/view/viewMetrics/trackFirstHidden.ts	100.00% <ø> (ø)
...on/view/viewMetrics/trackInteractionToNextPaint.ts	100.00% <ø> (ø)
...ion/view/viewMetrics/trackCumulativeLayoutShift.ts	94.73% <94.73%> (ø)
.../src/domain/rumEventsCollection/view/trackViews.ts	100.00% <100.00%> (ø)
...lection/view/viewMetrics/trackCommonViewMetrics.ts	100.00% <100.00%> (ø)
...tion/view/viewMetrics/trackFirstContentfulPaint.ts	100.00% <100.00%> (ø)
...lection/view/viewMetrics/trackFirstInputTimings.ts	100.00% <100.00%> (ø)
...ection/view/viewMetrics/trackInitialViewMetrics.ts	100.00% <100.00%> (ø)
... and 4 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more