-
Notifications
You must be signed in to change notification settings - Fork 24
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
Conversation
a5c79f2
to
bbe7d6e
Compare
…ate-Selenium-Tests
…ate-Selenium-Tests
…ate-Selenium-Tests
…ate-Selenium-Tests
…ate-Selenium-Tests
…ate-Selenium-Tests
There was a problem hiding this 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).
There was a problem hiding this 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.
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? |
There was a problem hiding this 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.
@JFU-GIT I tried running the tests w/ mock SLS.
Is there some other configuration I might be missing? |
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. |
@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?
|
@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" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this 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!
…ate-Selenium-Tests
There was a problem hiding this 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
There was a problem hiding this 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!
thanks team for multiple round of review and feedbacks, the feedbacks lead to many improvements which make a difference! |
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:
** 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)
Follow up work
** Set up a Jenkins job to perform the tests
** Note: May have to add or enhance a build container to add this Jenkins job
** 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?
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:
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:
Submitter Checklist
I have gone through and verified that...:
README
updates and changelog / release notes entries.TODO
and/orFIXME
comments, which include a JIRA ticket ID for any items that require urgent attention.