Skip to content

Conversation

@alfredeen
Copy link
Member

@alfredeen alfredeen commented Sep 16, 2025

Description

This PR fixes two failing integration tests. One test lacked proper visibility verifications around a model and nested button. The other test failed caused by missing required app description. Also added many should('be.visible') clauses to make tests more robust and also cleaned up some comments. I also converted one obsolete test to test that redeploy of shiny apps works in regards to the app status when changing the docker image. This test may fail until the app status code fixes this.

For some background, see Jira link

Checklist

  • This pull request is against develop branch (not applicable for hotfixes)
  • I have included a link to the issue on GitHub or JIRA (if any)
  • I have included migration files (if there are changes to the model classes)
  • I have included, reviewed and executed tests (unit and end2end) to complement my changes
  • I have updated the related documentation (if necessary)
  • I have added a reviewer for this pull request
  • I have added myself as an author for this pull request
  • In the case I have modified settings.py, then I have also updated the studio-settings-configmap.yaml file in serve-charts

@alfredeen alfredeen self-assigned this Sep 16, 2025
@alfredeen alfredeen added the enhancement Improvement of existing feature or request label Sep 16, 2025
@alfredeen alfredeen marked this pull request as ready for review September 17, 2025 14:51
@alfredeen alfredeen requested a review from a team as a code owner September 17, 2025 14:51
@alfredeen alfredeen requested a review from churnikov September 17, 2025 14:52
Copy link
Contributor

@churnikov churnikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll be honest, I did not really read into it, because it seems like it's mostly style changes and added .should("be.visible") calls.
So I don't have much to say here, and if tests pass, then lgtm.

One request though, I don't see in the readme instructions on how to run integration tests. Could we add it there?

@alfredeen
Copy link
Member Author

alfredeen commented Sep 18, 2025

I'll be honest, I did not really read into it, because it seems like it's mostly style changes and added .should("be.visible") calls. So I don't have much to say here, and if tests pass, then lgtm.

Yes there are many such changes. I also added a test that reproduces some shiny proxy app status problems. This test may now intentionally fail before a future fix is in place.

One request though, I don't see in the readme instructions on how to run integration tests. Could we add it there?

I see that our README has no mention of tests at all. I will add a section and mention this.

@alfredeen alfredeen requested a review from churnikov September 18, 2025 14:43
README.md Outdated

### End2End tests

UI end2end tests and Integration tests use the [Cypress](https://www.cypress.io/) framework and are implemented in javascript. The UI end2end are automatically executed as part of git workflows. You may also run them manually in this manner after first installing Cypress on your machine:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's worth mentioning that this requires a running instance of serve on a local machine.

@alfredeen alfredeen merged commit 7d5445a into develop Sep 19, 2025
2 checks passed
@alfredeen alfredeen deleted the integration-test-improvements branch September 19, 2025 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Improvement of existing feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants