Skip to content

UoE/Fix github action issues#2

Merged
milanmajchrak merged 54 commits intodatashare-UoEMainLibrary-dspace-8_xfrom
uoe/fix-github-actions
Apr 20, 2026
Merged

UoE/Fix github action issues#2
milanmajchrak merged 54 commits intodatashare-UoEMainLibrary-dspace-8_xfrom
uoe/fix-github-actions

Conversation

@milanmajchrak
Copy link
Copy Markdown
Collaborator

References

Add references/links to any related issues or PRs. These may include:

  • Fixes #issue-number (if this fixes an issue ticket)
  • Requires DSpace/DSpace#pr-number (if a REST API PR is required to test this)

Description

Short summary of changes (1-2 sentences).

Instructions for Reviewers

Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.

List of changes in this PR:

  • First, ...
  • Second, ...

Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • My PR passes ESLint validation using yarn lint
  • My PR doesn't introduce circular dependencies (verified via yarn check-circ-deps)
  • My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

Copilot AI review requested due to automatic review settings April 8, 2026 09:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@milanmajchrak milanmajchrak changed the base branch from main to datashare-UoEMainLibrary-dspace-8_x April 8, 2026 09:38
@milanmajchrak milanmajchrak force-pushed the uoe/fix-github-actions branch from a1bd2c6 to 52ef654 Compare April 8, 2026 11:12
…emove CI-FIX-README

- Restore build.yml to base (Node [18.x, 20.x], e2e/SSR/Docker, --code-coverage)
- Only build.yml change: --browsers=ChromeHeadlessCI for CI stability
- Revert themed-copyright/organised selectors to ds-themed-* with eslint-disable
- Remove CI-FIX-README.md
- Restore package.json to base (--code-coverage intact)
- Keep karma.conf.js ChromeHeadlessCI launcher + timeout settings
Revert ChromeHeadlessCI launcher and timeout overrides in karma.conf.js.
Revert build.yml test command to base (yarn run test:headless).
CI uses NODE_OPTIONS='--max-old-space-size=4096' from env which is sufficient.
DataShare fork has significantly more code/tests than upstream DSpace.
The upstream 4096MB is insufficient — only ~2500 of 5000+ tests compile
with 4GB heap due to webpack bundle size. Increase to 8192MB to ensure
all tests compile and code coverage processing completes.
Coverage data collection from Chrome takes longer than the default 30s
timeout on large projects. Set to 600s (10 min) to allow full coverage
data transfer for 5000+ test files.
…ble-dev-shm-usage

Root cause: With --code-coverage, Chrome crashes mid-execution due to
/dev/shm exhaustion on CI runners. Standard CI fix: --disable-dev-shm-usage
routes shared memory to /tmp instead.

Changes:
- karma.conf.js: Add ChromeHeadlessCI launcher with --no-sandbox and
  --disable-dev-shm-usage; keep browserNoActivityTimeout for coverage data
- package.json: Use ChromeHeadlessCI in test:headless
- build.yml: Revert NODE_OPTIONS to upstream 4096 (4GB sufficient on Linux CI)
Chrome's default V8 heap (~1.5GB) is insufficient for the coverage-
instrumented test bundle — Chrome crashes after ~60% of tests.
Adding --js-flags=--max-old-space-size=4096 gives Chrome enough
heap for the full test suite with code coverage.
With the DataShare fork having ~25% more test code than upstream,
Chrome needs more V8 heap for coverage-instrumented execution.
Linux overcommit allows 8GB allocation on 7GB runner — only
actively used pages need physical memory.
With coverage instrumentation, total memory (Node ~4GB + Chrome ~3GB) exceeds
the default GitHub Actions runner's 7GB RAM. Adding 4GB swap provides enough
headroom for both processes during code coverage test execution.
Combine 8GB Node heap with 4GB swap for reliable code coverage tests.
Use dd fallback if fallocate fails. Print free -h for debugging.
Source maps double the coverage-instrumented bundle size, causing Chrome
to crash after ~60% of tests. Disabling source maps in CI (they're not
needed for automated testing) reduces the bundle enough for Chrome to
handle the full test suite.

Also revert NODE_OPTIONS to upstream 4096 and remove swap step
(swap didn't help — issue was bundle size, not system memory).
Using npx ng test directly instead of yarn run test:headless to avoid
the possibility of --source-map=false being ignored when appended
to the npm script's --source-map=true.
After 10+ CI attempts with various approaches (8GB Node heap, Chrome V8
heap increase, 4GB swap, --disable-dev-shm-usage, --source-map=false),
code coverage consistently crashes Chrome after ~60% of tests.

Root cause: Istanbul coverage instrumentation creates a bundle too large
for Chrome on the 7GB GitHub Actions runner. Upstream DSpace works because
their test suite is ~50% smaller.

Changes:
- build.yml: Run ng test directly without --code-coverage
- build.yml: Add if-no-files-found: ignore for coverage artifact upload
- karma.conf.js: Simplified ChromeHeadlessCI (removed unneeded flags)
- package.json: test:headless still has --code-coverage for local use
The direct 'yarn ng test' command fails on CI in 2s. Use the package.json
'test' script instead, which already has --source-map=true, --watch=false,
and --configuration test. Add --browsers=ChromeHeadlessCI for headless mode.
Coverage (--code-coverage) is kept in test:headless for local use.
… builds

Restore --disable-gpu, --js-flags=--max-old-space-size=4096 in ChromeHeadlessCI,
and browserDisconnectTimeout/Tolerance settings that were present in the green
builds. These were accidentally removed during code coverage debugging.
Restores the test command to use the test:headless npm script, which
includes --code-coverage. The ChromeHeadlessCI custom launcher provides
--no-sandbox, --disable-gpu, and 4GB V8 heap for the large test suite.
…tion

After all tests complete, Istanbul coverage data serialization can take
significant time on the large DataShare test suite. 120s was insufficient;
increase to 300s to allow time for coverage processing.
The mock was returning only { detect: spy } but the component accesses
updateFileNames, fileNamesSignal, hasUploadFilesErrorsSignal, getDuplicates,
and getDuplicateFileNamesDisplay. This caused afterAll errors that crashed
Chrome after test completion.
Upstream DSpace GHCR images (ghcr.io/dspace/*) are private to the DSpace
org. The fork's GITHUB_TOKEN cannot access them. DockerHub images are
public and contain the same tags (dspace-8_x-test, dspace-8_x-loadsql, etc).

Comment out DOCKER_REGISTRY env var so docker-compose defaults to docker.io.
Remove the now-unnecessary docker/login-action step.
… sections open)

The DataShare signal-based one-section-at-a-time behavior prevented section
content from rendering, causing submission e2e tests to fail when trying to
interact with form elements inside closed panels.
The DataShare fork hardcoded 'Subject Keywords' as the visible text for the
Subject filter, but left the aria-label using the i18n translation key
('Subject'). This mismatch violates WCAG 2.5.3 label-content-name-mismatch
(axe-core) because the accessible name must contain the visible text.

Fix: revert to upstream i18n pattern for the visible text, and update the
en.json5 translation key to 'Subject Keywords' to preserve DataShare's
desired display text. Both aria-label and visible text now derive from
the same i18n key, eliminating the mismatch.
- recent-item-list: remove role='link' from <button> as it violates
  WCAG - buttons must not have link role (axe: aria-allowed-role)
- community-list-page: change <h1> to <h2> to improve heading
  hierarchy and avoid h1 duplication with theme chrome header
@milanmajchrak milanmajchrak force-pushed the uoe/fix-github-actions branch from c62cdc1 to f492cb5 Compare April 13, 2026 15:08
- Restore ds-themed-about selector with eslint-disable (review point 7)
- Restore model.context.label in form Add buttons (review point 12)
- Restore signal-based one-section-at-a-time accordion (review point 14)
- Update Cypress submission tests for one-at-a-time accordion behavior
- Preserve a11y improvements (aria-labels, aria-hidden)
model.context might be undefined for default DSpace form array models
that don't set context.label. Use optional chaining (?.) to prevent
runtime crash, with fallback to form.add translation.
@milanmajchrak milanmajchrak merged commit 353baae into datashare-UoEMainLibrary-dspace-8_x Apr 20, 2026
9 checks passed
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.

2 participants