Arvados

developer-run-tests-services-workbench2: #889

21811-side-favorites-test @ arvados|7223714878b7ff520c58261d12e0b3e9f5aa5c8d

• ✅ All agreed upon points are implemented / addressed.
• ✅ Anything not implemented (discovered or discussed during work) has a follow-up story.
• ✅ Code is tested and passing, both automated and manual, what manual testing was done is described
  no manual testing was done
• n/a Documentation has been updated.
• ✅ Behaves appropriately at the intended scale (describe intended scale).
• n/a Considered backwards and forwards compatibility issues between client and server.
• ✅ Follows our coding standards and GUI style guidelines.

• The new test runs twice because that is what would have caught the bug resolved in #21712. Also fixed a failing banner test that had previously been pushed to main.

Sorry for not looking into this earlier. In the future feel free to remind me of pending reviews at standup regardless of the review task setting.

I think the assertions could be more thorough, for example asserting that "myFavoriteProjectN" doesn't exist before expanding "My favorites" and also after collapsing it, and the same with public favorites. That way, we make sure all the behaviors are well tested.

Apart from that, the rest LGTM, thanks!

developer-run-tests-services-workbench2: #1010

21811-side-favorites-test @ 5a286de98a23f50bd1ee98a5ab2238c8117bcbe7

I think the assertions could be more thorough...

Done

I'm seeing a couple issues with the last commit 5a286de98 :

• The commit message's first line only includes the ticket number. I think it should also include the description that's below it.
• The it.only() call should be removed so that all tests are executed.

With that, it'll LGTM. Thanks!