Add e2e tests for the host details view #728
Conversation
rtorrero
requested review from
dottorblaster,
fabriziosestito,
nelsonkopliku and
stefanotorresi
|
@rtorrero awesome! How about we meet tomorrow 15m before the daily standup and you show me exactly what the test covers? I'll send you and invite. BTW, I've just noticed that the links to Azure components (vm and resource group) are no longer enabled. Therefore yes, the test would simply consist on checking the SAP system link and the cluster link. |
There was a problem hiding this comment.
Great Job so far @rtorrero! Really happy to see how well you onboarded on this! 🚀
Just a couple of comments that I hope might be useful.
rtorrero
force-pushed
the
e2e_test_host_detail
branch
from
05b5973
to
ea43573
Compare
|
I've added some additional checks requested by @abravosuse and adjusted the code a bit to follow @nelsonkopliku suggestions. Should be good now but let me know if I missed anything |
There was a problem hiding this comment.
Hey @rtorrero !
Maybe they are planned for future PRs but, I would try to include the:
- SUSE subscriptions table test
- Trento agent status test
- The
Hosts > vmhdbprd01text test (just after the title) - Include some
Not foundtest, but I guess this PR only includes happy path tests
If they are not coming in this PR, everything else looks pretty good!
| describe("Detailed view for a specific host should be available", () => { | ||
| it("should show the host I clicked on in the overview", () => { | ||
| cy.get( | ||
| ".border-top > :nth-child(1) > .col-sm-12 > .row > :nth-child(1) > .text-muted" |
There was a problem hiding this comment.
I'd consider adding some more specific css class in order to simplify selector. Something like a container tn-host-detail-container and then the name in a child element like tn-hostname
That would simplify selecting .tn-host-detail-container > .tn-hostname
What do you think?
There was a problem hiding this comment.
Addressing comments by @arbulu89 and @nelsonkopliku should be more than enough. Brilliant! LGTM
rtorrero
force-pushed
the
e2e_test_host_detail
branch
from
ea43573
to
cb3059e
Compare
rtorrero
force-pushed
the
e2e_test_host_detail
branch
from
cb3059e
to
1b7283d
Compare
|
Everything has been addressed with the exception that as you suggested, we are going for happy path only in this tests. We don't want to get too deep in testing all elements of the view as we expect that lots of these things are going to change soon; at least that's what we agreed with @abravosuse |
There was a problem hiding this comment.
Great job @rtorrero! 🚀
There's still some selectors that might be improved application wide, which would require refactoring different views and related tests, but we can address those later by iterating.
For me it is a green light.
This is another PR that introduces end to end tests for the host details view. I've written the Gherkin from the test scenarios described by @abravosuse
I followed the document strictly where only a few aspects of the view are checked, such as the SAP System SID or cluster name, but I'd like see what you think and if we should check everything or just a few things here.
Also notice that in the
Backgroundsection on thehost_details.featureI describe the whole 27-hosts deployment as I'm reusing it from the hosts overview tests. I'm unsure if we should only mention in the background the aspects of the fixtures that we use and how it would look.