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

Add support for playwright e2e tests #1954

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

c0d33ngr
Copy link

@c0d33ngr c0d33ngr commented Dec 10, 2023

PR to close issue #1650

@CLAassistant
Copy link

CLAassistant commented Dec 10, 2023

CLA assistant check
All committers have signed the CLA.

@vishal423
Copy link
Contributor

@c0d33ngr, could you please resolve conflicts to allow GitHub Actions to trigger. Further, I suggest enabling the tests in one of the GitHub actions so that we can verify the tests execution result.

@c0d33ngr
Copy link
Author

Alright @vishal423 I'll address both issues. Thank you for your patience.

@c0d33ngr
Copy link
Author

async function loginByApi(request, username, password) {

const loginResponse = await request.get('api/authenticate', {
    form: true,
    body: {
        username,
        password,
        'remember-me': true,
    }
});
expect(loginResponse.status()).toEqual(200);
const csrfCookie = loginResponse.headers()['set-cookie'].split(';')[0].split('=')[1];
console.log(username);
console.log(password);

const loginResponseWithCookie = await request.post('api/authentication', {
    form: true,
    headers: {
        'X-XSRF-TOKEN': csrfCookie,
    },
    body: {
        username,
        password,
        'remember-me': true,
    },
});

console.log(loginResponseWithCookie);
expect(loginResponseWithCookie.status()).toEqual(200);
const accountResponse = await request.get('api/account');
expect(accountResponse.status()).toEqual(200);

}

Hi @vishal423 this snippet of code is a rough draft of the loginByApi function with csrfcookie. The expect(loginResponseWithCookie.status()).toEqual(200); throws 403 status code. The username and password both are correct credentials with cookie too

You mind giving any suggestions on how to go about it?

@vishal423
Copy link
Contributor

The expect(loginResponseWithCookie.status()).toEqual(200); throws 403 status code.

403 points to CSRF validation failure. I suspect the cookie that you pass along authenticate request is not valid. If you take a look at cypress equivalent commands, we read the csrf value from cookie name XSRF-TOKEN

return cy.getCookie('XSRF-TOKEN').should('have.property', 'value')

As a side note, In the initial request (GET), you don't need to pass form/parameters as that's used to retrieve the authenticated user info/csrf cookie.

@c0d33ngr c0d33ngr changed the title Add some playwright-e2e tests conversion Add support for playwright e2e tests Jan 3, 2024
@vishal423
Copy link
Contributor

@c0d33ngr, do you plan to continue on this PR?

@c0d33ngr c0d33ngr marked this pull request as ready for review March 24, 2024 09:23
@c0d33ngr
Copy link
Author

@c0d33ngr, do you plan to continue on this PR?

Yes I intend on doing so.

@vishal423
Copy link
Contributor

@c0d33ngr, Thanks. Lets scope test scenarios excluding entity in this PR. Let me know if you also want to scope authentication type (between JWT, Session, OAuth2) in this initial PR.

Meantime, I will start on moving cypress under a specific option/flag so that we can give an option to choose between cypress and playwright (later once becomes stable).

@c0d33ngr
Copy link
Author

@c0d33ngr, Thanks. Lets scope test scenarios excluding entity in this PR. Let me know if you also want to scope authentication type (between JWT, Session, OAuth2) in this initial PR.

Meantime, I will start on moving cypress under a specific option/flag so that we can give an option to choose between cypress and playwright (later once becomes stable).

Okay. entity could be in another PR same as authentication scope. The only issue I'm having in this PR is fixing the merge conflict from my end

@vishal423
Copy link
Contributor

ok. I will take a close look over the upcoming weekend.

Couple of initial observations:

  • All templates files should be moved to generators/client/templates instead of generators/client/templates/svelte
  • I couldn't understand the use of generators/client/templates/svelte/playwright-e2e/jdl-config.js contents. We use ejs templates to access user selected or default options during code generation and the variables inside your file should be accessible to you as those are in other template files (cypress or others).
    e.g. Within ejs script, the variable variable databaseTypeSql with appropriate value should be available and you don't need to add new logic to create or populate this to use in your scripts.
  • We also need update to package.json scripts to run playwright tests. Currently we use following for cypress, so, you can create equivalent for playwright:
	"e2e": "npm run e2e:ci -- --headed",
	"e2e:ci": "cypress run --browser chrome",
        "cy:open": "cypress open --e2e --browser chrome",

@c0d33ngr
Copy link
Author

@c0d33ngr, Thanks. Lets scope test scenarios excluding entity in this PR. Let me know if you also want to scope authentication type (between JWT, Session, OAuth2) in this initial PR.

Meantime, I will start on moving cypress under a specific option/flag so that we can give an option to choose between cypress and playwright (later once becomes stable).

Okay. entity could be in another PR same as authentication scope. The only issue I'm having in this PR is fixing the merge conflict from my end

ok. I will take a close look over the upcoming weekend.

Couple of initial observations:

* All templates files should be moved to `generators/client/templates` instead of `generators/client/templates/svelte`

* I couldn't understand the use of `generators/client/templates/svelte/playwright-e2e/jdl-config.js` contents. We use [`ejs`](https://ejs.co/) templates to access user selected or default options during code generation and the variables inside your file should be accessible to you as those are in other template files (cypress or others).
  e.g. Within ejs script, the variable variable `databaseTypeSql` with appropriate value should be available and you don't need to add new logic to create or populate this to use in your scripts.

* We also need update to [package.json](https://github.com/jhipster/generator-jhipster-svelte/blob/main/generators/client/templates/package-template.json.ejs) scripts to run playwright tests. Currently we use following for cypress, so, you can create equivalent for playwright:
	"e2e": "npm run e2e:ci -- --headed",
	"e2e:ci": "cypress run --browser chrome",
        "cy:open": "cypress open --e2e --browser chrome",

Thank you for the observations.

  • About the jdl-config.js file, it was created so I could use the predefined user selected or default options to substitue the ejs but I would have to look through it to use it in playwright with little help from you if the need arises
  • About the package.json file you pointed me to, I'll create equivalent playwright commands in this PR

@vishal423 vishal423 marked this pull request as draft May 26, 2024 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants