-
Notifications
You must be signed in to change notification settings - Fork 1
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
Activity Report Stepper cucumber tests #53
Conversation
…into kw-cucumber-js
…art-TTADP into kw-cucumber-test-js
…into kw-cucumber-test-js
…ad-Start-TTADP into kw-cucumber-test-js
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 like how the tests scenarios flow in a understandable way! Have a few questions, nothing major.
src/middleware/authMiddleware.js
Outdated
@@ -20,8 +20,13 @@ export const hsesAuth = new ClientOAuth2({ | |||
* @param {*} res - response | |||
*/ | |||
export function login(req, res) { | |||
const uri = hsesAuth.code.getUri(); | |||
res.redirect(uri); | |||
if (req.headers.cookie && req.headers.cookie === `CUCUMBER_USER=${process.env.CUCUMBER_USER}`) { |
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.
Do we want to guard this with a feature flag so we can make sure it isn't used production?
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 CUCUMBER_USER is a secret var, but I will add an extra check whether we are in production
Then('the {string} button is disabled', async (string) => { | ||
const page = scope.context.currentPage; | ||
const buttonOneSelector = 'button[disabled]'; | ||
// const buttonOne = await select(page).getElement('button:contains("Previous")'); |
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.
Can remove some dead code
docs/adr/0010-bdd-testing.md
Outdated
|
||
## Context | ||
|
||
Behavior-driven development (BDD) allows for a broader team to collaborate on software development. The business stakeholders have insight into how well the software meets the requirements. Through automated tests they have a way to validate the functionality in a user friendly way. Cucumber, and it particular cucumber-js was selected as the tool of choice to accomplish this. To go along with cucumber-js, puppeteer as well as selenium webdriver were looked at to provide a full solution for BDD testing. |
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.
Cucumber, and it particular cucumber-js was selected as the tool of choice to accomplish this
Feels like this should be in the Decision
section and is missing the selection of puppeteer.
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 point. The idea was to present why we chose Puppeteer, since it was compared to some extent with the selenium webdriver. I didn't look at alternatives to Cucumber itself. I did reorganize the sections though to hopefully make it clearer. Hope it makes more sense now.
docs/adr/0010-bdd-testing.md
Outdated
|
||
## Decision | ||
|
||
Puppeteer as well as selenium webdriver provide a way to run automated browser tests. Both of these are popular tools to enable browser automation. Puppeteer offers more control over Chromium based browsers, very fast, headless by default, run modej, and it can take screenshots of the pages, both in an image and PDF formats. Additionally, it measures rendering and load times by Chrome Performance Analysis tool and it is easier to set up than selenium webdriver. The drawback to using Puppeteer is it's lack of full cross-browser support, which is offered by the selenium webdriver, since Puppeteer is specialized for Chromium. |
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 paragraph feels like it should be in the context section. Also this talks about Puppeteer and Selenium but not cucumber.
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.
Same as above.
docs/adr/0010-bdd-testing.md
Outdated
|
||
## Context | ||
|
||
Behavior-driven development (BDD) allows for a broader team to collaborate on software development. The business stakeholders have insight into how well the software meets the requirements. Through automated tests they have a way to validate the functionality in a user friendly way. Cucumber, and it particular cucumber-js was selected as the tool of choice to accomplish this. To go along with cucumber-js, puppeteer as well as selenium webdriver were looked at to provide a full solution for BDD testing. |
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.
Behavior-driven development (BDD) allows for a broader team to collaborate on software development. The business stakeholders have insight into how well the software meets the requirements. Through automated tests they have a way to validate the functionality in a user friendly way. Cucumber, and it particular cucumber-js was selected as the tool of choice to accomplish this. To go along with cucumber-js, puppeteer as well as selenium webdriver were looked at to provide a full solution for BDD testing. | |
Behavior-driven development (BDD) allows for a broader team to collaborate on software development. The business stakeholders have insight into how well the software meets the requirements. Through automated tests they have a way to validate the functionality in a user friendly way. Cucumber, and in particular cucumber-js was selected as the tool of choice to accomplish this. To go along with cucumber-js, puppeteer as well as selenium webdriver were looked at to provide a full solution for BDD testing. |
docs/adr/0010-bdd-testing.md
Outdated
|
||
## Decision | ||
|
||
Puppeteer as well as selenium webdriver provide a way to run automated browser tests. Both of these are popular tools to enable browser automation. Puppeteer offers more control over Chromium based browsers, very fast, headless by default, run modej, and it can take screenshots of the pages, both in an image and PDF formats. Additionally, it measures rendering and load times by Chrome Performance Analysis tool and it is easier to set up than selenium webdriver. The drawback to using Puppeteer is it's lack of full cross-browser support, which is offered by the selenium webdriver, since Puppeteer is specialized for Chromium. |
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.
Puppeteer offers more control over Chromium based browsers, very fast, headless by default, run modej, and it can take screenshots of the pages, both in an image and PDF formats.
Should it be run mode
or run model
? Not sure what you are trying to say there.
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.
Thanks. It's run mode
. I'll update
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.
Looks good!
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.
Nothing to change right now, but I had some feedback for future cucumber tests and steps.
@@ -0,0 +1,28 @@ | |||
Feature: Activity Report Stepper |
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 format of these steps is perfect. Good job
# Scenario: Login is redirected to HSES | ||
# Given the home page of tta-smarthub | ||
# When pressing login | ||
# Then we should see "Head Start Enterprise System" page |
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'd prefer this test was deleted instead of being commented out. Please do that when the stepper steps are removed.
|
||
Then('I moved past the {string} step', async (string) => { | ||
const page = scope.context.currentPage; | ||
assert.equal(string, 'Activity Summary'); |
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 line will lock down this step to only work with "Activity Summary". If that's desired, we should hard-code that in the test definition instead of using the parameter. If we want it to work with other strings then this line should be removed.
I also think a lot of these steps could be combined down to a single "Then I am on the {string} step". It would result in slightly worse readability in the feature file, but I think that is worth reducing duplicated code here. Another option is extracting the test steps into a function and calling that from each version of the step, or getting creative with regexes to match more versions of the feature file to a single step definition.
No need to change these steps at this time though.
await page.waitForSelector(selector); | ||
const value = await page.$eval(selector, (el) => el.textContent); | ||
|
||
assert.equal(value, string); |
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.
For the next time someone tries to use this step: this test should try to find a link by the text and then assert that it found anything. Or the name should be updated to pass in the href or other way to select more general links than only the activity-reports
link.
Activity Report Stepper cucumber tests (#53)
Description of change
This PR adds tests for the Activity Report Stepper. To enable tests of the stepper, the auth is bypassed by setting a cookie containing a secret user id during the headless puppeteer tests.
How to test
.env.example
.yarn deps
and thenyarn start:local
yarn cucumber
reports
. This can be seen on the artifacts tabIssue(s)
Checklist