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

BB2-545-Create-Selenium-Tests #932

Merged
merged 44 commits into from
Jul 21, 2021
Merged

Conversation

JFU-GIT
Copy link
Contributor

@JFU-GIT JFU-GIT commented Jun 7, 2021

JIRA Ticket:
BB2-545

User Story or Bug Summary:

As a BB2 developer, it is highly desirable to automate test run of BB2 components that involve human end user interaction, e.g. the built-in testclient, account management, app creation and revision, etc.

Selenium webdriver API provides framework and browser plugins and python plugin to help:
(1) Record user interaction of a functional task and export into python code.
(2) Trim and customize the recorded python code into BB2 auto run client tests.

A/C:

  • Add full cycle tests, e.g. automated headless run of BB2 testclient leveraging selenium webdriver
    ** Provide coverage of auth flow
    ** Provide coverage of FHIR flow
    ** Provide coverage of other flows, e.g. account management flow (implementation in separate tickets to be opened)
    ** Provide coverage of site static pages e.g. no links are broken, logos are properly shown etc. (implementation in separate tickets - to be opened)
  • Deliver local documentation on how to run these tests
  • Add any follow up work discovered to the list below 

Follow up work

  • When this scope is completed, every developer can run post-deployment automated tests against every environment 
    ** Set up a Jenkins job to perform the tests
    ** Note: May have to add or enhance a build container to add this Jenkins job
  • Future discovery on automated browser tests in New Relic 
    ** Note: New Relic can use this scope as a reference for what to test but not HOW to test it 

What Does This PR Do?

  • Implemented test harness base on Selenium webdriver
  • Provided tests for BB2 built-in test client:
    • V1/V2 authorization flow tests
    • Demo Access Grant or Deny
    • FHIR Resources read / search Patient, EOB, Coverage, Meta Data, Profile, OIDC Discovery info

When testing with SLSX as identity service, need to provision redirect url on SLSX client manager.

note: the redirect url used by selenium tests already provisioned on TEST

What Should Reviewers Watch For?

If you're reviewing this PR, please check these things, in particular:

  • Does the harness makes it easy to test BB2 logic where webUI involved?

  • Are the testclient based auth fow and data flow test (v1/v2 + slsx/mslsx) cases provide good coverage of major bb2 flows?

  • Is it easy to add in more webUI involved BB2 functional areas, e.g. account management, etc.?

  • Are any additional documentation updates needed?

  • Are there any unhandled and/or untested edge cases you can think of?

  • Can you find any bugs if you run the code locally and test it manually?

  • Is it easy to be integrated into GH Action?

  • TODO

What Security Implications Does This PR Have?

Submitters should complete the following questionnaire:

  • If the answer to any of the questions below is Yes, then here's a link to the associated Security Impact Assessment (SIA), security checklist, or other similar document in Confluence: N/A.
    • Does this PR add any new software dependencies? Yes (Selenium).
    • Does this PR modify or invalidate any of our security controls? No.
    • Does this PR store or transmit data that was not stored or transmitted before? No.
  • If the answer to any of the questions below is Yes, then please add @StewGoin as a reviewer, and note that this PR should not be merged unless/until he also approves it.
    • Do you think this PR requires additional review of its security implications for other reasons? No.

What Needs to Be Merged and Deployed Before this PR?

This PR cannot be either merged or deployed until the following pre-requisite changes have been fully deployed:

  • CMSgov/some_repo#42

Submitter Checklist

I have gone through and verified that...:

  • This PR is reasonably limited in scope, to help ensure that:
    1. It doesn't unnecessarily tie a bunch of disparate features, fixes, refactorings, etc. together.
    2. There isn't too much of a burden on reviewers.
    3. Any problems it causes have a small "blast radius".
    4. It'll be easier to rollback if that becomes necessary.
  • I have named this PR and its branch such that they'll be automatically be linked to the (most) relevant Jira issue, per: https://confluence.atlassian.com/adminjiracloud/integrating-with-development-tools-776636216.html.
  • This PR includes any required documentation changes, including README updates and changelog / release notes entries.
  • All new and modified code is appropriately commented, such that the what and why of its design would be reasonably clear to engineers, preferably ones unfamiliar with the project.
  • All tech debt and/or shortcomings introduced by this PR are detailed in TODO and/or FIXME comments, which include a JIRA ticket ID for any items that require urgent attention.
  • Reviews are requested from both:
    • At least two other engineers on this project, at least one of whom is a senior engineer or owns the relevant component(s) here.
    • Any relevant engineers on other projects (e.g. BFD, SLS, etc.).
  • Any deviations from the other policies in the DASG Engineering Standards are specifically called out in this PR, above.
    • Please review the standards every few months to ensure you're familiar with them.

@JFU-GIT JFU-GIT changed the title draft selenium tests BB2-545-Create-Selenium-Tests Jun 7, 2021
@JFU-GIT JFU-GIT force-pushed the jfuqian/BB2-545-Create-Selenium-Tests branch from a5c79f2 to bbe7d6e Compare June 7, 2021 05:11
@JFU-GIT JFU-GIT marked this pull request as ready for review June 16, 2021 23:09
Copy link
Contributor

@oragame oragame left a comment

Choose a reason for hiding this comment

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

I successfully run the selenium tests locally (MSLSX), I have no additional comments at this time but agree with what other comments have already been mentioned (and addressed as best I can tell).

Copy link
Contributor

@dtisza1 dtisza1 left a comment

Choose a reason for hiding this comment

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

Still working to test locally. Noticed one change so far.

docker-compose/bluebutton_server_start.sh Outdated Show resolved Hide resolved
@njdister
Copy link
Contributor

I have been able to get everything working locally with both MSLSx and real SLSx. This is not a huge issue, but I noticed there is a lot of container duplication between MSLSx and SLSx containers, when really the only thing that differs between them is parameters / environment variables. Is there a way to reduce container duplication in the docker compose file and simply pass the right parameters when run by the bash script?

Copy link
Contributor

@dtisza1 dtisza1 left a comment

Choose a reason for hiding this comment

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

This looks good to me!

I was able to get both types of tests working in my local setup today. For some reason, I couldn't get the slsx type tests to work on Friday.

For this to work and to switch between the types of tests, I have to completely clean out my previous docker setups. I think this is OK, since working with these tests won't be a frequent thing. This also seems to be something related to just my type of local env.

Wondering if Nathan's suggestion related to simplifying the docker compose setup would help my case? I don't think it's necessary to make any changes, if it adds a lot of extra time.

Good work on this! I like that both real and mock slsx/mslsx are being tested. The tests are very comprehensive. Excited to see how we utilize this for regular and scheduled testing of our full auth flows in the future!

Noticed some refactoring changes pushed up just now. Will wait for your final changes and take a last look before approving.

@nbragdon
Copy link
Contributor

@JFU-GIT I tried running the tests w/ mock SLS.
I seem to get timeouts for every test:

ERROR: test_auth_deny_fhir_calls_v1 (apps.integration_tests.selenium_tests.SeleniumTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/apps/integration_tests/selenium_tests.py", line 179, in test_auth_deny_fhir_calls_v1
    self._play(TESTS[test_name], step, api_ver=api_ver)
  File "/code/apps/integration_tests/selenium_tests.py", line 152, in _play
    self.actions[action](*s.get("params", []), **kwargs)
  File "/code/apps/integration_tests/selenium_tests.py", line 81, in _find_and_click
    elem = WebDriverWait(self.driver, timeout_sec).until(EC.visibility_of_element_located((by, by_expr)))
  File "/usr/local/lib/python3.8/dist-packages/selenium/webdriver/support/wait.py", line 80, in until
    raise TimeoutException(message, screen, stacktrace)
selenium.common.exceptions.TimeoutException: Message: 

Is there some other configuration I might be missing?
I just checked out this branch, and ran ./docker-compose/run_selenium_tests_local_keybase.sh

@JFU-GIT
Copy link
Contributor Author

JFU-GIT commented Jul 13, 2021

@JFU-GIT I tried running the tests w/ mock SLS.
I seem to get timeouts for every test:

ERROR: test_auth_deny_fhir_calls_v1 (apps.integration_tests.selenium_tests.SeleniumTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/apps/integration_tests/selenium_tests.py", line 179, in test_auth_deny_fhir_calls_v1
    self._play(TESTS[test_name], step, api_ver=api_ver)
  File "/code/apps/integration_tests/selenium_tests.py", line 152, in _play
    self.actions[action](*s.get("params", []), **kwargs)
  File "/code/apps/integration_tests/selenium_tests.py", line 81, in _find_and_click
    elem = WebDriverWait(self.driver, timeout_sec).until(EC.visibility_of_element_located((by, by_expr)))
  File "/usr/local/lib/python3.8/dist-packages/selenium/webdriver/support/wait.py", line 80, in until
    raise TimeoutException(message, screen, stacktrace)
selenium.common.exceptions.TimeoutException: Message: 

Is there some other configuration I might be missing?
I just checked out this branch, and ran ./docker-compose/run_selenium_tests_local_keybase.sh

could be caused by residue containers left over from previous run of different mode...., try to bring down the instances and do a containers cleansing between mslsx vs slsx.

@nbragdon
Copy link
Contributor

@JFU-GIT Maybe if having a clean environment is a requirement, we could just include that in the run of the selenium tests? Just so we don't have to manually do it every time, forget, etc.

@dtisza1
Copy link
Contributor

dtisza1 commented Jul 14, 2021

@JFU-GIT Maybe if having a clean environment is a requirement, we could just include that in the run of the selenium tests? Just so we don't have to manually do it every time, forget, etc.

@nbragdon @JFU-GIT I was having that same issue with the timeouts. Completely cleaning out my docker containers, etc. each time running a different test type (and initially) worked. For the other day I was OK doing this, but sometimes I might be working on something else where I need to keep my docker setups.

Wondering if this could be updated to not interfere (or be affected by) other existing containers?

@JFU-GIT Maybe if having a clean environment is a requirement, we could just include that in the run of the selenium tests? Just so we don't have to manually do it every time, forget, etc.

@JFU-GIT
Copy link
Contributor Author

JFU-GIT commented Jul 14, 2021

@nbragdon @dtisza1 with the latest optimization, there is no need to deep clean extra containers, when switch slsx vs mslsx, only need to do a docker-compose -f docker-compose.selenium.yml down which is light weight and necessary (to discard top containers baked with mode specific env vars...)

@JFU-GIT
Copy link
Contributor Author

JFU-GIT commented Jul 15, 2021

@njdister @nickrobison-usds @dtisza1 thanks for feedbacks, refactored the scripts, so that on each run re-build top level services (picking up env vars set by script), it does not add much lapsed time, about 20 sec+ on my laptop from start script till first test case kicking in...


echo_msg " - Removing files"
rm "${CERTSTORE_TEMPORARY_MOUNT_PATH}/ca.cert.pem"
rm "${CERTSTORE_TEMPORARY_MOUNT_PATH}/ca.key.nocrypt.pem"
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good to also stop all docker containers once the run is done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

on second though, might need to keep the containers to attach and trouble shoot...
e.g. check loggings, docker-compose -f docker-compose.selenium.yml logs > full_log.txt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

take that back, added logic to stop services, it's considered rare needing to do trouble shoot

SeleniumDockerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@njdister njdister left a comment

Choose a reason for hiding this comment

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

I'm able to run MSLSX and SLSX tests successfully, while also verifying nothing breaks with the existing local development docker compose workflow. Nice work!

Copy link
Contributor

@nbragdon nbragdon left a comment

Choose a reason for hiding this comment

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

Nice job on this 👍 , excited to get this running on our PR's

Copy link
Contributor

@dtisza1 dtisza1 left a comment

Choose a reason for hiding this comment

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

This is looking good to me!

@JFU-GIT Incredibly great work on this! I'm excited for us to utilize this in our PR checks and alerting.

I'm still having some timeout (selenium.common.exceptions.TimeoutException) issues after running one of the tests successfully and trying the other test type afterward. This isn't important to address in this PR and might only be affecting my type of setup. I am able to run the tests OK with a little cleanup!

This looks to solve the problem for me in between running mslsx/slsx type tests:

$ ./docker-compose/run_selenium_tests_local_keybase.sh
$ docker-compose -f docker-compose.selenium.yml down
$ docker rmi bluebutton-web-server_tests_w_slsx bluebutton-web-server_bb2slsx bluebutton-web-server_tests bluebutton-web-server_bb2
$ ./docker-compose/run_selenium_tests_local_keybase.sh slsx

Thank you!

@JFU-GIT
Copy link
Contributor Author

JFU-GIT commented Jul 21, 2021

thanks team for multiple round of review and feedbacks, the feedbacks lead to many improvements which make a difference!

@JFU-GIT JFU-GIT merged commit a257722 into master Jul 21, 2021
@JFU-GIT JFU-GIT deleted the jfuqian/BB2-545-Create-Selenium-Tests branch July 21, 2021 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Medium Medium Priority For Reviewing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants