-
Notifications
You must be signed in to change notification settings - Fork 37
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
SWC-6531: refactor SWC e2e helpers to use exposed SRC methods and remove duplicate code #5178
Conversation
…ove duplicate code
export enum BackendDestinationEnum { | ||
REPO_ENDPOINT, | ||
PORTAL_ENDPOINT, |
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.
Replicate BackendDestinationEnum
, so that the endpoint can be passed to doGet
, etc... without evaluating the page to access SRC.
export async function waitForSrcEndpointConfig(page: Page) { | ||
// window only available after page has initially loaded | ||
await navigateToHomepageIfPageHasNotBeenLoaded(page) | ||
// ensure that endpoint config is set, | ||
// ...so API calls point to the correct stack | ||
await expect(async () => { | ||
const response = await page.evaluate('window.SRC.OVERRIDE_ENDPOINT_CONFIG') | ||
expect(response).not.toBeUndefined() | ||
}).toPass() |
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.
Method moved from e2e/helpers/verification.ts, so it is colocated with other methods that rely on evaluating the page to access SRC
const srcEndpoint = await SRC.SynapseEnums.BackendDestinationEnum[ | ||
endpoint | ||
] | ||
// @ts-expect-error: Cannot find name 'SRC' | ||
return await SRC.HttpClient.doPost( | ||
url, | ||
requestJsonObject, | ||
accessToken, | ||
srcEndpoint, | ||
additionalOptions, | ||
) |
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.
Maintain a doPost
method that evaluates the page to call the SRC.HttpClient.doPost
method to reduce code duplication in API calls. Plan to remove this method (and doGet
and doDelete
) once SynapseClient is its own package that can be used directly instead
function getUserIdFromJwt(token: string) { | ||
const payload = JSON.parse( | ||
Buffer.from(token.split('.')[1], BASE64_ENCODING).toString(), | ||
) | ||
return payload.sub | ||
} |
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.
Moved from e2e/helpers/http.ts, since method is only used in this file
accessToken?: string, | ||
userName?: string, | ||
apiKey?: string, | ||
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.
Instead of passing a Page
around, you could consider configuring the test process to point to the correct stack using an environment variable, probably by reading the Maven settings.xml so there's just one source of truth. Then I think a lot of this code could run without requiring the browser and loaded page.
You wouldn't have confidence that the SRC.OVERRIDE_ENDPOINT_CONFIG
is properly set, but I don't think that's such a big deal.
What do you think?
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 that idea for looking up the correct stack! Since we'll still need to evaluate the page to use SRC's HttpClient and SynapseClient, I might hold off on implementing that for now. However, I've opened a ticket to track the work and capture this idea!
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.
Generally looks good, I had one substantial design question to think about
No description provided.