-
Notifications
You must be signed in to change notification settings - Fork 6
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
draft for STCS test cases #230
base: v2.x/rc
Are you sure you want to change the base?
Conversation
Signed-off-by: Himani1519 <[email protected]>
Signed-off-by: Himani1519 <[email protected]>
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.
Please have a look and update the code. Please let me know if you have any ques.
@@ -81,6 +86,11 @@ class ConnectionPage{ | |||
async isGreenCheckIconVisible() { | |||
return await this.greenCheckIconSelector.isHidden(); | |||
} | |||
|
|||
async click_resumeProgress(){ |
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 function should be in the Title page file and already exists so you can use it from 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.
Done
} | ||
} | ||
|
||
async clickOnContinueToUnpax(){ |
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.
As we have a separate page for Unpax, I think we can add all unpax-related stuff under unpax page file.
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.
We don't have unpax page as of 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.
Correct, we don't have the page with the title 'Unpax' but on the 'Installation Type' page we are getting the option to 'Continue to Unpax' page, and on that page, we are getting the option 'Skip Unpax' so I think that is a page but it doesn't have the specific title as the title depends on which Installation type we are selecting. So I think if we are following the Page object model then we should add the 'Unpax' page separately. Let me know your thoughts or we can discuss this with our QA team.
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 agree as your new PR contains unpax page so will wait for your PR to merge then after that I will fix them after taking pull, because writing same code in two files does not make sense.
this.mainXpath = '//html/body/div/div[2]/div/div[4]/div/form/div/div[3]/div[1]/div[3]/div/div[2]/div/div/div' | ||
this.stc_mainXpath = '//html/body/div[1]/div[2]/div/div[4]/div/form/div/div[3]/div[1]/div[4]/div/div[2]/div/div/div/' | ||
this.mainXpath = '//html/body/div/div[2]/div/div[4]/div/form/div/div[2]/div[1]/div[3]/div/div[2]/div/div/div' | ||
this.stc_mainXpath = '//html/body/div/div[2]/div/div[4]/div/form/div/div[2]/div[1]/div[4]/div/div[2]/div/div/div' |
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.
Avoid using dynamic xpaths which changes frequently
|
||
|
||
|
||
constructor(page: 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.
You have initialized locators under Constructor but you haven't added page objects for them under class. So if you see there are 20 page objects under class, but under Constructor there are more than 30 you initialized. And by using the same locators you have created methods so it does not make sense. It causes failure of tests. Please have a look and update them.
@@ -0,0 +1,254 @@ | |||
import { test, ElectronApplication, expect, _electron as electron } from '@playwright/test'; |
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.
All the test cases are failing for me due to the issues in the Page object file so needs to be repaired.
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.
Its running test cases selecting SMP/E installation type so whatever the runtime dir you are passing should be exist already on rs28. For me all test cases are getting passed.
playwright_test/Tests/Stcs.spec.ts
Outdated
const LOAD_LIB = process.env.LOAD_LIB; | ||
const AUTH_LOAD_LIB = process.env.AUTH_LOAD_LIB; | ||
const AUTH_PLUGIN_LIB = process.env.AUTH_PLUGIN_LIB; | ||
const UPLOAD_PAX_PATH = process.env.ZOWE_ROOT_DIR |
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.
Remove unused declaration.
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.
we are using them at this line https://github.com/zowe/zen/pull/230/files#diff-3ec40a03d28e9024612ae4af658741981a84c2c55f9e6a816f3f493218c5b590
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 am not able to see if you used UPLOAD_PAX_PATH anywhere.
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.
Done.
playwright_test/Tests/Stcs.spec.ts
Outdated
await page.waitForTimeout(30000); | ||
installationTypePage.clickSkipUnpaxButton() | ||
await page.waitForTimeout(2000); | ||
// installationTypePage.clickContinueToInstallation() |
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.
Remove commented code.
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.
Done
|
||
test('test all required fields', async ({ page }) => { | ||
await page.waitForTimeout(5000); | ||
await expect(stcsPage.zis).toBeTruthy() |
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.
Remove the unwanted 'await' statement as the 'await' does not affect the expect statements.
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.
Adding wait so that page loaded successfully and then we can check expected result.
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.
That is fine but I mean to say if you use await in expect statement, it does not affect the result. So if you see in the same file for many expect statements, you haven't used 'await'. So I was asking to remove it if it is not necessary.
Signed-off-by: Himani1519 <[email protected]>
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.
Approved but not ready to merge. We will merge after James's PR will get merged with updated test logics.
Proposed changes
This PR addresses Issue: [Link to Github issue within https://github.com/zowe/zen/issues if any]
This PR depends upon the following PRs:
Type of change
Please delete options that are not relevant.
PR Checklist
Please delete options that are not relevant.
Testing
Further comments