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

Smoke Tests fixes #2021

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from
Open

Smoke Tests fixes #2021

wants to merge 11 commits into from

Conversation

coverbeck
Copy link
Contributor

@coverbeck coverbeck commented Sep 18, 2024

Description
Auth smoke tests were failing on QA. Fixing it led to a chain of other things. The original problem was due to the versions table in QA now requiring scrolling at a lower resolution with the addition of the DOI column. Interacting with the Actions dropdown in Cypress near the edge of the viewport seemed to be flaky. This led to:

  • Increasing the resolution that tests are run in. See Slack discussion. Although this punts on the scrolling issue, we were also running at an odd resolution, so why fight it.
  • Some of the integration tests started failing after increasing the resolution, sigh. I think some of our tests are just flaky, and changing the resolution moved the flakiness around. Hopefully reduced flakiness:
    • Waiting for certain Ajax requests to complete before proceeding
    • Use trackBy in some template loops; this causes DOM elements to be reused instead of recreated. This fixed some Cypress errors about DOM elements no longer being present.
  • If the tests failed, the resetDockstoreTestUser4 step would not get executed. This would then cause the next run to fail, etc. Always have it execute, and for good measure, execute it before running the tests (in case somebody runs the tests from a local box and doesn't reset).

Review Instructions
Ensure that all the smoke tests are passing nightly for a couple of days in a row on CircleCI.

Issue
SEAB-6674

Security
If there are any concerns that require extra attention from the security team, highlight them here.

Please make sure that you've checked the following before submitting your pull request. Thanks!

  • Check that your code compiles by running npm run build
  • Ensure that the PR targets the correct branch. Check the milestone or fix version of the ticket.
  • If this is the first time you're submitting a PR or even if you just need a refresher, consider reviewing our style guide
  • Do not bypass Angular sanitization (bypassSecurityTrustHtml, etc.), or justify why you need to do so
  • If displaying markdown, use the markdown-wrapper component, which does extra sanitization
  • Do not use cookies, although this may change in the future
  • Run npm audit and ensure you are not introducing new vulnerabilities
  • Do due diligence on new 3rd party libraries, checking for CVEs
  • Don't allow user-uploaded images to be served from the Dockstore domain
  • If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.
  • Check whether this PR disables tests. If it legitimately needs to disable a test, create a new ticket to re-enable it in a specific milestone.

@coverbeck coverbeck self-assigned this Sep 18, 2024
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.61%. Comparing base (6a73bf0) to head (f208564).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2021      +/-   ##
===========================================
- Coverage    60.62%   60.61%   -0.02%     
===========================================
  Files          394      394              
  Lines        12315    12316       +1     
  Branches      2958     2958              
===========================================
- Hits          7466     7465       -1     
- Misses        4837     4839       +2     
  Partials        12       12              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -228,7 +227,7 @@ function testTool(registry: string, repo: string, name: string) {
});
}

function testWorkflow(registry: string, repo: string, name: string) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These parameters were not being used, so I just removed them. Their existence was throwing me off as I was trying to understand the code.

Copy link

sonarcloud bot commented Sep 20, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

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.

4 participants