Skip to content
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(quay): add first playwright tests #1201

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

jrichter1
Copy link
Contributor

Adds a playwright based test suite for quay plugin. This is to help with automation for features contributed by RHTAP.

Also adds a github action that runs said automation for any plugin that would have it as PR check.

@jrichter1 jrichter1 requested review from fmenesesg and a team as code owners February 13, 2024 08:33
@invincibleJai
Copy link
Member

/cc @karthikjeeyar
/cc @rohitkrai03

@invincibleJai
Copy link
Member

Thanks @jrichter1, This looks good. would be good to update CONTRIBUTING.MD with steps needed to run and provide test results in the PR description

Locally i needed to install playwright as well

yarn playwright install --with-deps chromium

name: Playwright tests

on:
pull_request:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as this workflow runs on pull_request so does that run for quay only if changes are in quay plugin or even if it's not changed?

Copy link
Contributor Author

@jrichter1 jrichter1 Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now it will run for all plugins that have the ui-test npm script, regardless of changes

I can try skipping unchanged plugins if this is an issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be nice if we can run It based on change

Copy link
Contributor Author

@jrichter1 jrichter1 Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the job to first look for any suitable plugins that have changed in the PR. Unchanged plugins should now be ignored, and the whole test run will be skipped if no changes in relevant plugins are detected (if it works properly).

@jrichter1 jrichter1 requested a review from a team as a code owner February 14, 2024 14:10
@jrichter1
Copy link
Contributor Author

Thanks @jrichter1, This looks good. would be good to update CONTRIBUTING.MD with steps needed to run and provide test results in the PR description

Locally i needed to install playwright as well

yarn playwright install --with-deps chromium

updated

});

test('Vulnerabilities are listed', async () => {
const severity = ['High:', 'Medium:', 'Low:'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is the data being fetched, will it change with time are always have these severities?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is being fetched from a quay repo that I've made for this reason. The way I see it, if the image doesn't get updated, the number of vulnerabilities should not decrease. As far as the test cares, the number just needs to stay above zero.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, it doesn't change with time then it should be good.

/**
* See https://playwright.dev/docs/test-configuration.
*/
export default defineConfig({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this config be at each plugin level or can it be at the app level as this would be similar for most plugins?

Copy link
Contributor Author

@jrichter1 jrichter1 Feb 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is going to be similar for most plugins. I've put it at the plugin level since it was indicated to me that plugins might change code bases in the future. This way it should be self contained and easy to move.

@invincibleJai
Copy link
Member

@subhashkhileri @josephca FYI

.github/workflows/pr-playwright.yaml Outdated Show resolved Hide resolved
Co-authored-by: Karthik Jeeyar <[email protected]>
@openshift-ci openshift-ci bot removed the lgtm label Feb 16, 2024
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@karthikjeeyar
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Feb 16, 2024
@kadel
Copy link
Member

kadel commented Feb 16, 2024

This is awesome @jrichter1 🎉

/approve

Copy link

openshift-ci bot commented Feb 16, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel, karthikjeeyar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit a7e936d into janus-idp:main Feb 16, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants