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

Add cypress support #1951

Merged
merged 3 commits into from
Jul 29, 2020
Merged

Conversation

dvkruchinin
Copy link
Contributor

Motivation and context

Adding cypress support. And automation of running tests in CI.

How has this been tested?

Manual testing.

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

@coveralls
Copy link

coveralls commented Jul 28, 2020

Pull Request Test Coverage Report for Build 6657

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.01%) to 64.997%

Files with Coverage Reduction New Missed Lines %
cvat/apps/engine/media_extractors.py 2 76.7%
Totals Coverage Status
Change from base Build 6649: 0.01%
Covered Lines: 11100
Relevant Lines: 16675

💛 - Coveralls

@nmanovic nmanovic requested a review from bsekachev July 28, 2020 19:17
@@ -8,14 +8,15 @@

describe('Check server availability', () => {
it('Server web interface is available', () => {
cy.visit('http://localhost:8080')
cy.visit('/')
cy.wait(30000)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the usual practice here? Could you please quote its documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cypress documentation about this: https://docs.cypress.io/api/commands/visit.html#Prefixes
I moved "baseUrl" parameter to the configuration file (cypress.json). This will make it easier to navigate in location, and if need to change it than only need to change it in one place.
Added "wait" for loading the preloader. This will help avoid errors in checking the availability of page elements when first visit the page after starting docker containers.
This may be reworked in the future when more tests are added.

Copy link
Member

Choose a reason for hiding this comment

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

@dvkruchinin

Hi, maybe try to play with visit arguments: retryOnStatusCodeFailure or retryOnNetworkFailure and timeout as cypress docs say:

https://docs.cypress.io/api/commands/visit.html#Arguments

Also from the docs about availability of page elements:
Can you see what Cypress is doing under the hood? It’s automatically waiting and retrying because it expects the content to eventually be found in the DOM. It doesn’t immediately fail!

https://docs.cypress.io/guides/getting-started/writing-your-first-test.html#Step-2-Query-for-an-element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, I removed the wait(). Locally, the test passed without this command.

@nmanovic
Copy link
Contributor

@dvkruchinin , could you please modify CONTRIBUTION.md guide and add information how to run cypress tests locally?

@dvkruchinin
Copy link
Contributor Author

@dvkruchinin , could you please modify CONTRIBUTION.md guide and add information how to run cypress tests locally?

Done. Added information.

Dmitry Kruchinin added 2 commits July 29, 2020 13:32
Modifications:
- .travis.yml: adding command for run testin via cypress
- Dockerfile.ci: addin install necessary package for correct cypress
works. Adding install cypress via npm.
- docker-compose.ci.yml: Starting necessary docker containers for
cypress testing.
- tests/cypress.json: Cypress configuration file
- tests/cypress/integration/auth_page.js: little bit fix.
- CONTRIBUTING.md: add documentation how to run cypress tests locally
bsekachev
bsekachev previously approved these changes Jul 29, 2020
@@ -0,0 +1,4 @@
{
"video": false,
"baseUrl": "http://cvat_ui"
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest setup default baseUrl to "http://localhost:3000", create another file (like cypress_ci.json) and add parameter --config-file=cypress_ci.json to the command on CI. In this case users do not need change something by default (like described in CONTRIBUTING.md)

https://docs.cypress.io/guides/references/configuration.html#Options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree, thanks. This is the more correct way. I'll do it now.

@nmanovic nmanovic merged commit 6eb777a into cvat-ai:develop Jul 29, 2020
@nmanovic nmanovic linked an issue Jul 29, 2020 that may be closed by this pull request
2 tasks
@dvkruchinin dvkruchinin deleted the dkru/add_cypress_support branch August 19, 2020 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI testing infrastructure
4 participants