Skip to content

Add e2e reaction test, improve accessibility, enable parallel testing#37081

Merged
silverwind merged 12 commits into
go-gitea:mainfrom
silverwind:e2e-reactions-and-random-string
Apr 3, 2026
Merged

Add e2e reaction test, improve accessibility, enable parallel testing#37081
silverwind merged 12 commits into
go-gitea:mainfrom
silverwind:e2e-reactions-and-random-string

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 2, 2026

Add a new e2e test for toggling issue reactions via the reaction picker dropdown.

Add aria-label attributes to improve reaction accessibility:

  • Add aria-label="Add reaction" to the reaction picker dropdown
  • Add role="group" with aria-label="Reactions" to the reactions container, giving it a semantic identity for screen readers
  • Include the reaction key in each reaction button's aria-label (e.g. +1: user1, user2) so screen readers announce which reaction a button represents

E2e test improvements:

  • Simplify randomString to use Math.random instead of node:crypto
  • Replace generatePassword with a static password, remove unused clickDropdownItem
  • Enable fullyParallel: true and workers: '50%' in Playwright config
  • Run both chromium and firefox in all environments (not just CI)
  • Parallelize login and apiCreateRepo setup where possible
  • Use dedicated test user in user-settings test for concurrency safety

This PR was written with the help of Claude Opus 4.6

Add a new e2e test for toggling issue reactions via the reaction
picker dropdown.

Extract `randomString` utility to replace ad-hoc `Date.now()` and
`Math.random()` ID generation across all e2e tests, fixing potential
name collisions when tests run in parallel workers.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 2, 2026
Comment thread tests/e2e/utils.ts Outdated
silverwind and others added 2 commits April 2, 2026 14:16
Improves accessibility by adding an `aria-label` to the reaction
picker dropdown, making it identifiable by screen readers and
semantic test selectors.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
Add `role="group"` with `aria-label="Reactions"` to the reactions
container and include the reaction key in each button's aria-label.
Update the e2e test to use semantic selectors.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
silverwind and others added 3 commits April 2, 2026 16:39
Replace crypto-based hex generation with simple alphanumeric
character selection, and reuse `randomString` in `generatePassword`.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
Replace randomString calls with deterministic names derived from
the test name. Remove randomString, generatePassword, and unused
clickDropdownItem from e2e utils.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind changed the title Add e2e reaction test and randomString utility Add e2e reaction test and refactor e2e code Apr 2, 2026
silverwind and others added 3 commits April 3, 2026 00:48
Enable fullyParallel and run chromium+firefox simultaneously.
Add randomString suffixes to all test identifiers for concurrency
safety. Parallelize login and API setup where possible. Use
dedicated user in user-settings test to avoid admin state races.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@silverwind silverwind changed the title Add e2e reaction test and refactor e2e code Add e2e reaction test, improve accessibility, enable parallel testing Apr 2, 2026
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 2, 2026

Now using fullyParallel and workers: 50% in playwright which parallelizes both across browsers and within the same file. I've enabled Firefox now locally as well.

Total make test-e2e time is the same as before locally at ~9 seconds. All tests were verified to be concurrency-safe.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 3, 2026
@@ -1,5 +1,5 @@
{{if ctx.RootData.IsSigned}}
<div class="item action ui dropdown jump pointing top right select-reaction" data-action-url="{{.ActionURL}}">
<div class="item action ui dropdown jump pointing top right select-reaction" data-action-url="{{.ActionURL}}" aria-label="{{ctx.Locale.Tr "repo.add_reaction"}}">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Written by AI? Why here is "Add reaction" but not "Reactions"?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It is this button:

Image

It also has a toggle behaviour so can remove reactions as well, so you are right the label is not accurate. I have plans for this button to make it more like GitHub, e.g. inside the content btw.

Copy link
Copy Markdown
Member Author

@silverwind silverwind Apr 3, 2026

Choose a reason for hiding this comment

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

Changing to aria-label="Reactions" duplicates that label on the page making semantic selector unusable and my prompt was for semantic selectors only in that test, so I have to turn on of them non-semantic.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

With dbfec11, the button selector uses .select-reaction class, best we can do without introducing a different translation. Will be fixed later.

Use the existing "repo.reactions" translation for the reaction
picker dropdown, removing the now-unused "repo.add_reaction" key.

Co-Authored-By: Claude (Opus 4.6) <noreply@anthropic.com>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 3, 2026
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 3, 2026
@silverwind silverwind enabled auto-merge (squash) April 3, 2026 16:49
@silverwind silverwind disabled auto-merge April 3, 2026 16:49
@silverwind silverwind enabled auto-merge (squash) April 3, 2026 16:49
Comment thread tests/e2e/utils.ts Outdated
Signed-off-by: silverwind <me@silverwind.io>
@silverwind silverwind merged commit d80640f into go-gitea:main Apr 3, 2026
26 checks passed
@silverwind silverwind deleted the e2e-reactions-and-random-string branch April 3, 2026 17:20
@GiteaBot GiteaBot added this to the 1.26.0 milestone Apr 3, 2026
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 3, 2026
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 4, 2026
* main:
  Update `setup-uv` to v8.0.0 (go-gitea#37101)
  Fix various bugs (go-gitea#37096)
  Clean up AppURL, remove legacy origin-url webcomponent (go-gitea#37090)
  Add e2e reaction test, improve accessibility, enable parallel testing (go-gitea#37081)
  Fix various legacy problems (go-gitea#37092)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants