-
Notifications
You must be signed in to change notification settings - Fork 253
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
Readme for the Testing - detailed instructions for functional testing usage and addition of new tests #983
Conversation
a88ad41
to
810dd67
Compare
9bc2d11
to
89490a5
Compare
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.
The instructions look good, but this branch needs a rebase and there are unit tests that are currently failing
Pushed to halibut so that we have more time to make content revisions and complete a review. |
89490a5
to
0f9b1c4
Compare
In the instructions where Pester is installed and you have prerequisites, you are missing a step to install Selenium. That is before running the Update Selenium script. |
I noticed in the section where we tell the reader to "kill any ChromeDriver.exe processes", I found that those processes are sometimes hidden under the Powershell app and you cannot locate them in the main process list. Although the ChromeDriver.exe is not shown in my screenshot below, that is where I saw an instance of it earlier. Add this information to the instructions. |
Today when I tried running the functional tests, I was a getting a random error (forgot to take a screenshot) that was unrelated to any of our code. Selenium just wasn't working right for whatever reason and I cannot recall this ever happening before. I updated the ChromeDriver.exe but that didn't fix the problem so I ended up having to do what's below and now I can run the orchestrator again:
|
The README created for this PR is oriented to users (i.e. developers, testers) that want to execute the test orchestrator. I think you should add a couple of sentences at the top that makes this more obvious and change the title of the document to ScubaGear Functional Testing Automation for Developers and Testers. On that note, I had developed a couple of ancillary guides for us admins which are related to setting up the infrastructure (service principals, user provisioning, etc.). Can we add a link to those GitHub issues in this document for easy reference?
|
In your Functional Testing Usage section make it clear that the addition of the Thumbprint field to the -Data parameter is what makes the orchestrator run in service principal mode and that the thumbprint must correspond to the end user's client certificate which should be installed on the machine running the test plans. See #592 for example. I would just copy the following sections verbatim because they contain a lot of details and some of that stuff is missing from your README. |
Throughout the document I think it would be better to use the voice "you need to do this" instead of the third person "the user should do this". This makes the document easier to follow as a set of instructions directed at the reader. |
Add a section with the Powershell code to generate a user client certificate.
|
Replace "AAD functional testing in service principal mode by filtering specific test cases" with "When developing or if you are testing pull requests you may need to execute just a single baseline policy in the respective YAML test plan file. To do that you will add Pester filter tags to the PesterConfig parameter. See an example to execute the policy MS.AAD.2.1v1 test cases below." |
"Product functional tests are being run for multiple product-tenant combinations. Development team should check the successful completion of these runs for any regressions. " It is not clear when the development team should check this? Is this more the job of the operations team? Also, for performing regression tests, one option that a developer can exercise is to run the nightly product functional test via the GitHub actions against their branch although since that runs all of the products and all of the test cases, I think we should determine a more efficient way to do that if we ever want this option. |
I don't see a reference to the sample functional test execution script that is reference in this comment. That might make it easier for developers. |
Co-authored-by: James Garriss <[email protected]>
7419206
to
e930f6a
Compare
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 work. I have previously gone through the documentation and suggested tweaks. The overall team is doing a dry run of this document for release functional testing right now.
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.
Good work!
Co-authored-by: James Garriss <[email protected]>
e930f6a
to
1e482f9
Compare
Co-authored-by: James Garriss <[email protected]>
Co-authored-by: James Garriss <[email protected]>
Co-authored-by: James Garriss <[email protected]>
Co-authored-by: James Garriss <[email protected]>
Co-authored-by: James Garriss <[email protected]>
🗣 Description
Create a high-level Readme for the testing folder with detailed instructions for:
💭 Motivation and context
closes #735
🧪 Testing
✅ Pre-approval checklist
✅ Pre-merge checklist
PR passed smoke test check.
Feature branch has been rebased against changes from parent branch, as needed
Use
Rebase branch
button below or use this reference to rebase from the command line.Resolved all merge conflicts on branch
Notified merge coordinator that PR is ready for merge via comment mention
✅ Post-merge checklist