Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Infrastructure UI] Functional tests for basic functionality on Hosts View #147691

Closed
neptunian opened this issue Dec 16, 2022 · 14 comments · Fixed by #154026
Closed

[Infrastructure UI] Functional tests for basic functionality on Hosts View #147691

neptunian opened this issue Dec 16, 2022 · 14 comments · Fixed by #154026
Assignees
Labels
beta Required for a feature to move to beta Feature:Metrics UI Metrics UI feature Feature:ObsHosts Hosts feature within Observability info-needed Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services

Comments

@neptunian
Copy link
Contributor

neptunian commented Dec 16, 2022

After all features are in for the MVP, write basic functional tests similar to what we have for existing views, that loads some archived data and tests components are loading and possible user flows.

These tests could include:

  • Table exists and lists expected hosts with correct metrics
  • Query bar exists and lists and works when filtering for a host
  • Filtered results should be reflected across all the page content (Alerts, Metrics, Etc)
  • KPI Summary metrics exist with correct metrics
  • Charts exist with correct time series data
  • AlertSummary and table exist with correct data
  • Logs exists and show correct entries
  • ✔ Technical Preview behaviour (Done in #148235)
@neptunian neptunian added Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services Epic: Host Observability labels Dec 16, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@roshan-elastic roshan-elastic added Feature:ObsHosts Hosts feature within Observability and removed Epic: Host Observability labels Jan 22, 2023
@roshan-elastic
Copy link

Hey @neptunian - still want to do this?

Just figuring out priorities of this...(cleaning up the backlog)

@roshan-elastic
Copy link

Hey @neptunian - just pinging this one...

@neptunian
Copy link
Contributor Author

@roshan-elastic Yes.

@roshan-elastic
Copy link

Cheers @neptunian - would you be able to quickly recap the benefits of this (just for my understanding)?

I'm guessing it's best practice, will help us maintain quality and making testing quicker every time we deploy to it (but will prob some up-front work and some maintenance as we develop it).

Would that be about right?

@neptunian
Copy link
Contributor Author

neptunian commented Feb 28, 2023

@roshan-elastic Yes. We do this for all of our views and they are usually added in the same PR as the introduction of the feature without a separate issue necessary. Since this started as a POC and things were changing a lot it seemed more appropriate to wait until things stabilized so we don't have to rewrite these.

Typically we have unit tests for pieces of code that can be tested in isolation and then we have functional tests which actually start a browser and mimic a user clicking around. We make sure the basic things work although it can be detailed as well for important features. It helps to catch bugs that might appear once things are actually working together that we may not have thought about or we can't really test for otherwise. It's also like a basic "sanity" test to make sure the app functions at a basic level. For example a bug, perhaps unrelated to our code in one of the components we are using could cause the app not to load or the query bar to be broken for some reason, and this could catch that. CI would not allow the PR to go through.

We used to do manual testing before each release for a week or so with the whole team testing each other's work. We're trying to save time by automating this as much as possible with these tests.

@tonyghiani
Copy link
Contributor

Hey @neptunian, I've been following this issue since I was working on some functional tests for the Hosts View Alerts section.

Some basic tests were already implemented and with these changes we refactored and added some more tests for the page, but I believe we are still missing many more (such as testing the filtering options work fine, query bar etc), I'd be happy to help on this if there are more required tests!

@neptunian
Copy link
Contributor Author

@tonyghiani That would be great!

@roshan-elastic
Copy link

Great - thanks @neptunian . Sounds like we should try and do this if we're going to need to do this as part of moving to beta so moving this up the list.

@tonyghiani tonyghiani self-assigned this Mar 9, 2023
@tonyghiani
Copy link
Contributor

@neptunian I updated the description with some extra things that would need testing, would you add anything else or can we consider this as ready?

@neptunian
Copy link
Contributor Author

Thanks @tonyghiani! I think that looks good for now and we can consider this ready

@mohamedhamed-ahmed
Copy link
Contributor

@tonyghiani if this ticket will include the Functional Tests for the logs I guess we might need to add some extra points like:

  • Logs Query bar exists and filters logs correctly
  • A link to Logs Stream exists and navigates properly to the logs stream page

wdyt? feel free to add more if you see needed

@crespocarlos
Copy link
Contributor

Just a thought: In the future, maybe we could try to create the tests in the same PR that implements the feature, so we don't keep adding tech debts to this list.

@neptunian
Copy link
Contributor Author

@crespocarlos Agree. I was explaining in a comment above that ideally we do it this way, but it was put off until the product stabilized starting from the POC, though we probably could have started a bit earlier.

@roshan-elastic roshan-elastic added the beta Required for a feature to move to beta label Mar 24, 2023
tonyghiani added a commit that referenced this issue Mar 30, 2023
## 📓 Summary

Closes #147691 

- Test hosts table content.
- Test to verify the search results hosts to propagate the filter to
other sections.
- Test Logs tab existence, to add a test for logs content once
#154030 is resolved.

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
jgowdyelastic pushed a commit to jgowdyelastic/kibana that referenced this issue Mar 30, 2023
…54026)

## 📓 Summary

Closes elastic#147691 

- Test hosts table content.
- Test to verify the search results hosts to propagate the filter to
other sections.
- Test Logs tab existence, to add a test for logs content once
elastic#154030 is resolved.

---------

Co-authored-by: Marco Antonio Ghiani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta Required for a feature to move to beta Feature:Metrics UI Metrics UI feature Feature:ObsHosts Hosts feature within Observability info-needed Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants