[Spike] Review FE unit test issues and review progress

[JunTaoLuo]

Background/Request

As of 9/6/2024, the overall VAOS coverage statistics are as follows:

Although this meets the original epic goal, we would still have room for improvement. However, the previous issues filed for adding unit tests need to be review to ensure they are still relevant. One key consideration is that we are launching appointment details redesign in August after which many of the older components will be outdated and removed. We want to ensure we don't spend time adding unit tests for these soon to be obsolete components.

This will have significant overlap with https://github.com/department-of-veterans-affairs/va.gov-team/issues/84657 which tracks the removal of obsolete source code.

Goal

• Hit > 95% coverage metrics.

Requirements to Consider

• For each component to be tested:
  • Will this component still be needed after launching the appointment details redesign
  • Update the screenshot of the current coverage statistics for the component
  • Even if the coverage statistics show 100%, we still need to ensure tests for that component is added.
  • Note that many components may achieve a 100% coverage even without unit tests since a parent component might instantiate it in the parent's unit test. However, we still want to add more granular unit tests for the child components since parent components rarely test for the behavior of child components they instantiate.

Tasks

• Run yarn test:coverage-app --app-folder vaos in the root directory to generate the most up to date VAOS coverage statistics
• Review the unit test issues attached to the epic: https://github.com/department-of-veterans-affairs/va.gov-team/issues/67329
• Check each component and file new unit test issues if no unit tests are written and/or if coverage is less than 100%

Time Box

_ hours

Definition of Done

• Tasks are complete
• Research and recommendations are documented in this ticket

[Bren22va]

Hey team! Please add your planning poker estimate with Zenhub @cferris32 @jenniemc @JunTaoLuo @ryanshaw @simiadebowale @vbahinwillit

[JunTaoLuo]

Here are my current thoughts on this issue and I'm curious what folks think.

Given that we have already achieved 80% coverage mandated by the VA, it seems like the team is happy to deprioritize the work for additional test coverage. See below for current coverage:

In this spirit, I propose we close out the 23 unit-test issues that we currently have open to declutter our planning board. Instead we'll replace these with 4 issues that track a 4 phase approach to further improve our unit test coverage:

1. Add unit tests for areas that have < 80% statements coverage. This corresponds to the areas whose row is highlighted in red/yellow and comprise 7 items.
2. Add unit tests for areas that have < 80% coverage for any metric. This corresponds to areas that has red/yellow boxes and comprises ~17 items.
3. Add unit tests for areas that have < 100% coverage for any metric. This comprises >=~20 items.
4. Re-evaluate further improvements for test coverage. I expect this to enter this phase way out in the future and we'll likely have different requirements by then. Also, my understanding is that even though some components are "covered" as evaluated by this coverage tool due to coverage by parent components, they may still need individual tests to test functionality at a more granular level. Thus, we'll need to ensure each file/component has associated tests.

Thoughts?

[vbahinwillit]

I was thinking that it might be better to cleanup unit/e2e tests via cleanup of obsolete feature flags. I just went through that process when removing the 'va_online_scheduling_appointment_details_redesign' feature flag and found myself deleting or updating quite a few tests.

I don't remember all of the details on when feature flags are to be removed but we have quite a few that are 100% enabled in production. Also, removal of feature flags is part of our process.

[JunTaoLuo]

I absolutely agree, in fact I was able to close one of the unit test ticket after your PR was merged. Regarding the Flipper toggles, I filed a spike ticket to review our feature flags for that exact purpose because one thing I like more than writing code is deleting code :smile:. However, even when we have removed all of the obsolete code/feature toggles I expect that there will still be a set of tests we need to add/update. When that time comes, I think we should take a phased approach so the work is more structured, which is what I proposed here.

[JunTaoLuo]

Oh and one more thing. Even though I've mentioned this in standup, I'm going to add a note here that I expect us to delete quite a bit of code as part of https://github.com/department-of-veterans-affairs/va.gov-team/issues/84657 so that will help us figure out which tests we need as well.