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

fix: Updates to RStudio Integration tests #818

Conversation

nguyen102
Copy link
Contributor

@nguyen102 nguyen102 commented Nov 19, 2021

Issue #, if available:

Description of changes:

  • Remove extraneous comments
  • Don't run rstudio tests if rstudio config is 'N/A'
  • Move advanced tests into common folder so they get executed

Checklist:

  • Have you successfully deployed to an AWS account with your changes?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully tested with your changes locally?
  • If new dependencies have been added, have they been pinned to specific versions?
  • Is this change also required on the AWS Solution version?
  • Have you updated openapi.yaml if you made updates to API definition (including add, delete or update parameter and request data schema)?
  • If you had to run manual tests, have you considered automating those tests by adding them to end-to-end tests?
  • If you are updating the changelog and vending out a new release, have you updated versionNumber and versionDate in .defaults.yml

AS review ticket id:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nguyen102 nguyen102 requested a review from a team as a code owner November 19, 2021 04:14
maghirardelli
maghirardelli previously approved these changes Nov 19, 2021
@codecov
Copy link

codecov bot commented Nov 19, 2021

Codecov Report

Merging #818 (9b67a86) into develop (bb9e457) will not change coverage.
The diff coverage is n/a.

❗ Current head 9b67a86 differs from pull request most recent head 7078b6d. Consider uploading reports for the commit 7078b6d to get more accurate results
Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #818   +/-   ##
========================================
  Coverage    50.91%   50.91%           
========================================
  Files          286      286           
  Lines        15931    15931           
  Branches      2485     2485           
========================================
  Hits          8111     8111           
  Misses        6863     6863           
  Partials       957      957           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb9e457...7078b6d. Read the comment docs.

SanketD92
SanketD92 previously approved these changes Nov 19, 2021
@nguyen102 nguyen102 changed the title chore: Remove rstudio int test extraneous comments fix: Updates to RStudio Integration tests Nov 19, 2021
@nguyen102
Copy link
Contributor Author

Note: Before we can merge this PR in, we need to refactor the Cypress AppStream disabled workspace test to terminate workspaces that it launches. This is because the RStudio Integration test checks that all RStudio instances are terminated, before it runs.

An example of how to refactor the Cypress AppStream disabled workspace tests.

@SanketD92
Copy link
Contributor

Terminated RStudio instances manually on the e2e test environment. PR for automatic termination depends on this PR merge.

@SanketD92 SanketD92 merged commit eb879fe into awslabs:develop Nov 19, 2021
@nguyen102 nguyen102 deleted the remove-rstudio-int-test-extraneous-comments branch January 4, 2022 02:25
jxuamazon pushed a commit to jxuamazon/service-workbench-on-aws that referenced this pull request Feb 15, 2022
* chore: Remove extraneous comments from launch-rstudio-workspace.test.js

* fix: Don't run rstudio tests if rstudio config is 'N/A'

* Move advanced tests into common folder so they get executed

* Fix import path

Co-authored-by: Tim Nguyen <thingut@amazon.com>
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.

3 participants