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

Automated form list tests #79

Merged
merged 10 commits into from
Jan 18, 2024
Merged

Automated form list tests #79

merged 10 commits into from
Jan 18, 2024

Conversation

PascalinDe
Copy link
Member

This PR adds tests for the form list, including:

  • checking pagination
  • that forms titles are links to the forms
  • the vote/results buttons are displayed correctly

it does not for the moment contain internalization tests, which will be added later

closes #70

@PascalinDe PascalinDe self-assigned this Jan 11, 2024
@PascalinDe PascalinDe changed the title Playwright Automated form list tests Jan 12, 2024
Copy link
Member

@ineiti ineiti left a comment

Choose a reason for hiding this comment

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

Just some minor requests.

const user = sciper === SCIPER_USER ? User
: sciper === SCIPER_ADMIN ? Admin
: undefined;
const user = sciper === SCIPER_USER ? User : sciper === SCIPER_ADMIN ? Admin : undefined;
Copy link
Member

Choose a reason for hiding this comment

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

can you read and understand this in one breath?
I think superfluous parentheses would help me:

Suggested change
const user = sciper === SCIPER_USER ? User : sciper === SCIPER_ADMIN ? Admin : undefined;
const user = sciper === SCIPER_USER ? User : ( sciper === SCIPER_ADMIN ? Admin : undefined );

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree on the readability, but sadly this breaks the linting

Copy link
Member Author

Choose a reason for hiding this comment

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

added a commit w/ linting on that line disabled, so we can either use that or leave it like before

web/frontend/tests/formIndex.spec.ts Outdated Show resolved Hide resolved
@PascalinDe PascalinDe merged commit c8339fa into main Jan 18, 2024
11 checks passed
@PascalinDe PascalinDe deleted the playwright branch January 18, 2024 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

form list tests
2 participants