UoE/Fix github action issues#2
Merged
milanmajchrak merged 54 commits intodatashare-UoEMainLibrary-dspace-8_xfrom Apr 20, 2026
Merged
UoE/Fix github action issues#2milanmajchrak merged 54 commits intodatashare-UoEMainLibrary-dspace-8_xfrom
milanmajchrak merged 54 commits intodatashare-UoEMainLibrary-dspace-8_xfrom
Conversation
There was a problem hiding this comment.
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.
a1bd2c6 to
52ef654
Compare
…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
…orm Add button to i18n text
…to tag input, clean up diagnostics
…-group, remove axe diagnostic
…pt POST for collection-create timing
c62cdc1 to
f492cb5
Compare
…ix codecov fail_ci_if_error
- 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.
353baae
into
datashare-UoEMainLibrary-dspace-8_x
9 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
References
Add references/links to any related issues or PRs. These may include:
issue-number(if this fixes an issue ticket)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:
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!
yarn lintyarn check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.