Test DISCOVERY-437 (bonus: smoke test viewScanList!)

[bruno-fs]

What's included

Add a sanity check testing viewScanList and the behavior of the kebab
button;

Properly test DISCOVERY-437; given how tighly coupled everything is,
adding a view test was the only way to test it.

Notes

changing jest config and adding babel as transformer was required to be able to test those views - setting up those configs was way more challenging than actually writing those tests 😅

How to test

npm test

Coverage and basic unit test check

1. update the NPM packages with $ npm install
2. $ npm test
   -->

Example

...

Updates issue/story

DISCOVERY-437

[cdcabrera]

@bruno-fs the primary views were disabled from testing for multiple internal reasons discussed with @nicolearagao around how to architect UI code.

Any future "view" code actively needs to be broken apart into subsequent components, helpers, etc... before attempting to shoe-horn a testing solution into place. Some of the "many" future breakouts that need to happen include

• Toast notification alerts
• Table/inventory
• Moving API calls into the actual modals

...performing these types of updates will subsequently make testing the views easier. I recommend closing this PR down in the short term

[cdcabrera]

This work should not be merged until an overarching understanding of future UI architecture is discussed or understood

[bruno-fs]

That's an interesting point, @cdcabrera. While I agree the views (or at least this one) desperately need a refactor that makes them more decoupled - and at the very minimum easier to test, I don't think testing (at least smoke testing) should be postponed.

Waiting to test later is a liability and should be a team decision, not up to just you and @nicolearagao.

[cdcabrera]

The changes you are proposing here will

• create potential build conflicts
  • you're adding resources that were intentionally left out
• create potential redundancy with existing e2e tests
  • one of the reasons they were disabled to begin with
• testing may be able to be accomplished indirectly
  • testing support components may already cover the display facet
  • testing "resources/npms" should generally be unnecessary, those resources should already be tested, instead testing the configuration that feeds into those resources would be more beneficial
• these tests provide limited gains
  • these tests would most likely end up being replaced if the recommended approach is followed

The proper approach is still recommended, understanding the ui architecture. Any issues that arise from merging this work fall outside of my volunteer effort

[bruno-fs]

@mirekdlugosz it seems it won't be possible to test download functionality from scans page on this codebase.

I wrote this one because of your previous comment about having download functionality already covered on camayoc, but on the scan jobs modal... Do you have a suggestion on how to test this while the refactor doesn't happen?

[mirekdlugosz]

@bruno-fs I think we can cover that on Camayoc side. I put it on the list of things to do, but I was constantly busy with other UI work and now we have this rapidast integration on higher priority.

If you would want to try to implement it, it would look something like that:

Step one: Create a new test similar to test_download_scan. The key here is to use scans fixture.

It probably shouldn't have pr_only marker. The linked one has marker because it fundamentally does exactly the same thing as end to end test; the new one would use different action to download report, so there's a reason to run it on nightly.

In test you wouldn't use download_scan method, but download_scan_kebab? or download_scan should take a parameter to decide if it downloads through kebab or modal? or download_scan should use kebab, but there should be new method to download through modal? When making that decision, think that it's reasonable to expect Camayoc to also cover opening modal and downloading old report. Right now we don't test that and don't have any methods for that, but probably we should.

Also: right now only new UI gives two ways to download report, so this new test should run conditionally only if settings.camayoc.use_uiv2 is set to True. I think we are going to finally release new UI in a week or two, so I would probably not bother with conditional skips and just wait until new UI is the only one we have both upstream and downstream.

Step two: Add a new method to ScansMainPage class. For now you can have ScanListElem implementing a method that opens kebab, clicks "download" and closes kebab.

Unless you are feeling adventurous and want to implement generic kebab interaction in AbstractListItem class. Then I think specific implementations should have class attribute under specific name, where they can specify:

• action name, as understood by framework (so we don't have to change tests if ouiaid ever changes)
• ouiaid of item to click in kebab menu
• action? Some menu items only result in toast, other open modals. Generally a specific implementation should be able to tell what will happen, but I don't yet know how exactly it should look like.

So test / page method could do something like item.kebab_click(ITEM_ID), and abstract class would take care of opening the kebab, finding an item, clicking it and possibly responding to toast / modal.

When it comes to download specifically, we want longer longer timeout than default Playwright 30 seconds, and we manually refresh page on each step (I measured that the other day, manually refreshing gives much shorter tests execution time than waiting for UI to refresh automatically). You can copy the loop from existing implementation, although I guess there is an opportunity to abstract and re-use code.

[bruno-fs]

@cdcabrera just to be sure we are on the same page, if I remove all the proposed tests but this one (this was the true goal of testing here), would you still be against this PR?