Pyic 8272 - Playwright E2E POC #2258
Conversation
|
| await this.page.locator('#test_data').click(); | ||
| await this.selectTestData('Alice Parker (Valid) DVLA Licence'); | ||
| await this.setEvidenceScores({ | ||
| strengthHours: '3', |
There was a problem hiding this comment.
Why do these field names end in hours, minutes, and seconds?
| static getFeatureFlagsUrl(): string { | ||
| const baseUrl = this.getIdentityBuildUrl(); | ||
| const featureSet = this.getEnvironmentVariableOrDefault( | ||
| 'FEATURE_FLAGS', |
There was a problem hiding this comment.
I don't think this makes sense as an environment variable - we'll want different flags per tests
There was a problem hiding this comment.
Yeah you’re right. This is just a POC for this specific test. Once we start adding more tests, we can parameterise this variable so each test can pass in the feature flags it needs.
| TICF_API_KEY: ConfigurationReader.getTicfApiKey(), | ||
| }, | ||
| FEATURE_FLAGS: { | ||
| ENABLE_URL: ConfigurationReader.getFeatureFlagsUrl(), |
There was a problem hiding this comment.
Again, not sure a single feature flags URL will be useful for most tests.
| fraudPage: async ({ page }, use) => { | ||
| await use(new FraudStubPage(page)); | ||
| }, | ||
| apiService: async ({ request }, use) => { |
There was a problem hiding this comment.
This should be ticfApiService?
There was a problem hiding this comment.
thanks, I'll update.
| } | ||
|
|
||
| protected async clickButton(name: string): Promise<void> { | ||
| await this.page.getByRole('button', { name }).click(); |
There was a problem hiding this comment.
Would these be better off targetting an ID rather than a name?
| await this.clickButton('Continue'); | ||
| } | ||
|
|
||
| async navigateToDcmawSuccess(): Promise<void> { |
There was a problem hiding this comment.
Why do we need to navigate to this page? The end user will get there by clicking a button or link won't they?
| import { BasePage } from './base-page'; | ||
| import { CONFIG } from '../config/test-config'; | ||
|
|
||
| export class IdentityPage extends BasePage { |
There was a problem hiding this comment.
This class seems to contain methods relating to more than one page - should they be split into separate page classes?
| } | ||
|
|
||
| async expectIPVSuccess(): Promise<void> { | ||
| await this.expectHeading('Continue to the service you need to use'); |
There was a problem hiding this comment.
Would it be better to confirm that we're on the success page URL rather than look for text that might change in the future?
| async expectReuseScreen(): Promise<void> { | ||
| await expect(this.page.locator('#header')).toContainText('You have already proved your identity'); | ||
| await this.expectText('ALISON JANE PARKER'); | ||
| await this.expectText('80TYEOMAN WAYTROWBRIDGEBA14'); |
There was a problem hiding this comment.
Why are there missing spaces here?
| } | ||
|
|
||
| async getUserId(): Promise<string> { | ||
| return await this.page.getByRole('textbox', { name: 'Enter userId manually' }).inputValue(); |
There was a problem hiding this comment.
Again can we use an ID or similar rather than text?
(This applies to other place too, I won't comment on all of them)
|
Proposed changes
What changed
Why did it change
Issue tracking
Checklists