-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
test: Add initial automated UI smoke tests (#4393) #4694
Conversation
Looking, sorry for the delay. |
@keithchong I ran into some issue. What do I miss? Thanks.
|
@wtam2018 , just wanted to confirm and rule it out, but did you run npm install and npm compile ? |
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.
Sorry for the ridiculously long delay. Had to prioritize 1.8 bug fixing. Added few minor comments and one main proposal to use Page Object pattern for tests implementation.
Let me know what you think please. Happy to work together on initial implementation in your fork.
ui-test/package-lock.json
Outdated
@@ -0,0 +1,1904 @@ | |||
{ |
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.
Since we are using yarn
everywhere else, can you please generate yarn.lock instead?
@@ -0,0 +1,40 @@ | |||
# |
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 consistency with backend e2e tests please rename this folder to test/smoke
.
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 ui-test folder is at the 'top level', as a sibling to the ui folder because both are Node projects. Moving this to test/smoke will mean these tests will be 'mixed in' with the go test cases at a lower level. Should we keep those tests homogeneous?
@@ -0,0 +1,298 @@ | |||
import Configuration from './Configuration'; |
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 a big fan of page object pattern. The idea is to group page-specific methods into a class that represents a page and class instance can be created by navigating to the page so that test look as following:
// initializes web driver and returns instance of `Navigation` class that holds`WebDriver` instance
const navigation = utils.init();
// navigates to `/applications` page, waits for navigation to complete and returns `ApplicationsList` class
const appListPage = navigation.goToAppList();
// clicks new app buttons and returns `NewApplicationDialog` that holds link to sliding panel element
const newAppDialog = await appListPage.newAppButtonClick();
// locates app name element and sends 'myApp' keys
await newAppDialog.setAppName('myApp');
...
ApplicationsList:
export class ApplicationsList {
panel: WebElement;
public async setAppName(val: string) {
findUiElement(Const.CREATE_APPLICATION_FIELD_APP_NAME);
await appNameField.sendKeys(appName);
}
}
This approach was very helpful in one of my previous projects and helped us to effectively structure test utility functions.
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.
Hi @alexmt, what's the naming convention for file names in ArgoCD? For example,
export class VersionPanel extends React.Component<VersionPanelProps, {copyState: CopyState}> { |
version-info-panel.tsx
and the class name defined is VersionPanel
. Although, not all files contain classes is probably one reason why VersionPanel.tsx is not used (I'm thinking Java, of course).
I'll try to follow the same structure as in the actual code, eg. I'll create the applications-list
folder that contains the applications-list.ts
file.
Signed-off-by: Keith Chong <[email protected]>
955ec08
to
1ba3668
Compare
The latest changes now include the Page object design pattern. I've added some extras features:
All tests should now start with something like:
and end with To run the default test (this will change when we use mocha)
I still have one outstanding todo w.r.t. the review comments, if my counter-argument is given a thumbs-down:
Some outstanding things to consider in future PRs:
|
This PR depends on: |
Hi @alexmt , is this ok to be merged? The latest changes have to be reviewed. |
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.
Thank you @keithchong ! LGTM
Created followup ticket #6847 to work on Docker file to package tests dependencies.
Signed-off-by: Keith Chong [email protected]
#4393
All changes in this PR are in a new folder
argoproj/argo-cd/ui-test
, which parallelsargoproj/argo-cd/test
. This is an 'independent' package that resides in argo-cd.To try it out, ensure you have the changes to the following PR (or update to master since it is merged):
#4683
You can also simply get the contents of the ui-test folder on its own and put it anywhere on your file system.
Then, go to the ui-test folder and run (you need to have node.js installed):
ui-test/.env
(eg. Change IS_HEADLESS=true if you want to see the test run in the browser)Checklist: