464 implement frontend javascript testing by sbland · Pull Request #533 · direct-framework/direct-webapp

sbland · 2025-10-29T09:44:35Z

Description

Some tidying up of the frontend node code related to #529

Fixes #464

Type of change

Documentation (non-breaking change that adds or improves the documentation)
New feature (non-breaking change which adds functionality)
Optimization (non-breaking, back-end change that speeds up the code)
Technical work (non-breaking, change which is work as part of a new feature)
Bug fix (non-breaking change which fixes an issue)
Breaking change (whatever its nature)

Key checklist

All tests pass (eg. python -m pytest)
The documentation builds and looks OK (eg. mkdocs serve)
Pre-commit hooks run successfully (eg. pre-commit run --all-files)

Further checks

Code is commented, particularly in hard-to-understand areas
Tests added or an issue has been opened to tackle that in the future. (Indicate issue here: # (issue))

sbland · 2025-10-29T09:45:10Z

Fixes #464

codecov · 2025-10-29T09:45:37Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

sbland · 2025-10-29T09:52:14Z

@AdrianDAlessandro I moved the node documentation to a separate README.md inside the node directory. This seems to have broken the mkdocs build. I tried looking through the code and docs but I can't see how to add this file.

README.md

github-actions · 2025-11-27T19:38:21Z

README.md

-```bash
-npm run styles:expanded
-```
+See [node/README.md](https://github.com/direct-framework/direct-webapp/blob/main/node/README.md) for details on working with the NPM-based frontend asset pipeline.


🚫 [linkspector] _{reported by reviewdog 🐶}
Cannot reach https://github.com/direct-framework/direct-webapp/blob/main/node/README.md Status: 404

sbland · 2025-11-27T19:41:19Z

@AdrianDAlessandro FYI I've rebased this onto the main branch.

AdrianDAlessandro

I'll be happy to review and merge this if you can rebase it onto main. Would be good to merge this before looking at moving the node configs into a sub-directory

Co-authored-by: Adrian D'Alessandro <a.dalessandro@imperial.ac.uk>

sbland · 2026-03-02T16:43:33Z

@AdrianDAlessandro this is rebased back onto main and should be ready to merge

AdrianDAlessandro

Overall, this looks really good. I've run the tests and see that they pass. I haven't individually reviewed the tests because I am not likely to understand them. My only concern is that they might be too prescriptive. So if we make changes to the radial plots, it might cause many failures and make it difficult to update them all.

I've made some other comments, but they might be out of my naivety of best practice with node/npm/vitest, so please take it all with a grain of salt.

AdrianDAlessandro · 2026-03-09T15:33:05Z

node/README.md

+```bash
+npm run styles:expanded
+```
+


I think these lines are not necessary

Suggested change

```bash

npm run styles:expanded

```

This was copied from master README.md so happy to remove if no longer needed.

AdrianDAlessandro · 2026-03-09T15:40:07Z

package.json

+        "test": "vitest",
+        "test-ui": "vitest --ui",
+        "test-ci": "vitest --run"


Having the default test version be the interactive one is slightly confusing. I think maybe we should have the --run option be the default one and then have something like test-interactive be the one that persists and waits for file changes.

Also, the test-ui one failed for me because '@vitest/ui' is not installed. Not sure if we want to include it by default or not.

I think perhaps this is just a node default. I'm happy to change but I normally use the watch mode by default and have --run just for the CI.

Makes sense. Fine with me

AdrianDAlessandro · 2026-03-09T15:48:44Z

node/README.md

+To build all styles, scripts, and vendor files, run:
+
+```bash
+npm run build


I got some linting failures and a few file changes when I ran this.

Linting error:

$ npm run build > direct-webapp@0.1.0 build > npm-run-all --silent -p styles:expanded styles:minified scripts:expanded scripts:minified vendor --silent Vendor Info Bundling vendor scripts... StylesStyles InfoInfo Linting SCSS...Linting SCSS... Scripts Info Linting JavaScript... Scripts Info Linting JavaScript... Scripts Error! Scripts Error! /Users/adalessa/Projects/DIRECT/direct-webapp/node/src/js/dataviz/radial-plot-dataprocessing.js Scripts Error! 92:11 error Delete `··` prettier/prettier Scripts Error! 93:1 error Delete `··` prettier/prettier Scripts Error! Scripts Error! ✖ 2 problems (2 errors, 0 warnings) Scripts Error! 2 errors and 0 warnings potentially fixable with the `--fix` option. Scripts Error! Scripts ScriptsError! Linting errors found Error! Scripts Error! /Users/adalessa/Projects/DIRECT/direct-webapp/node/src/js/dataviz/radial-plot-dataprocessing.js Scripts Error! 92:11 error Delete `··` prettier/prettier Scripts Error! 93:1 error Delete `··` prettier/prettier Scripts Error! Scripts Error! ✖ 2 problems (2 errors, 0 warnings) Scripts Error! 2 errors and 0 warnings potentially fixable with the `--fix` option. Scripts Error! Scripts Error! Linting errors found Vendor Success Bundled vendor scripts successfully Vendor Info Bundling vendor styles... Vendor Success Bundled vendor styles successfully Styles Success Lint SCSS Styles Info Building compressed CSS... Styles Success Lint SCSS Styles Info Building expanded CSS... Styles Success Compile SCSS to CSS (compressed): Styles Success Compile SCSS to CSS (expanded): Browserslist: caniuse-lite is outdated. Please run: npx update-browserslist-db@latest Why you should do it regularly: https://github.com/browserslist/update-db#readme Browserslist: caniuse-lite is outdated. Please run: npx update-browserslist-db@latest Why you should do it regularly: https://github.com/browserslist/update-db#readme Styles Success Added vendor prefixes (Autoprefixer) Styles Success Added vendor prefixes (Autoprefixer)

Many of the changes were of this nature:

[data-bs-theme=dark] body.bg-secondary { - background-color: rgb(25.11, 28.02, 31.9) !important; + background-color: #191c20 !important; }

I think there is an issue with how it was set up first so I'll compare against a fresh install and see what is going on.

sbland · 2026-03-09T20:09:20Z

Overall, this looks really good. I've run the tests and see that they pass. I haven't individually reviewed the tests because I am not likely to understand them. My only concern is that they might be too prescriptive. So if we make changes to the radial plots, it might cause many failures and make it difficult to update them all.

I've made some other comments, but they might be out of my naivety of best practice with node/npm/vitest, so please take it all with a grain of salt.

Thanks @AdrianDAlessandro I'll take a look.

sbland mentioned this pull request Oct 29, 2025

Implement Frontend javascript Testing #464

Open

sbland requested a review from AdrianDAlessandro October 29, 2025 11:39

sbland self-assigned this Oct 29, 2025

sbland requested a review from davehorsfall October 29, 2025 11:39

AdrianDAlessandro reviewed Nov 19, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

github-actions bot reviewed Nov 27, 2025

View reviewed changes

sbland force-pushed the 464-implement-frontend-javascript-testing branch from f62472c to 053c4be Compare November 27, 2025 19:41

davehorsfall added this to Web App Development Dec 19, 2025

github-project-automation bot moved this to 📋 Backlog in Web App Development Dec 19, 2025

AdrianDAlessandro reviewed Feb 23, 2026

View reviewed changes

sbland and others added 10 commits March 2, 2026 16:35

Implement tests and some tidying up

3315fa6

Move frontend node docs from README.md to node/README.md

37533eb

Add docs about running node vitest

590fa1a

Update node readme

587f56d

Some linting amends

42161e8

Add tests for radial plot dataprocessing

9f09001

Add a github action to run node tests

d349fa9

Fix node version in ci

dd34685

Fix left .only in tests

9f49bdd

Update README.md

f7a61a7

Co-authored-by: Adrian D'Alessandro <a.dalessandro@imperial.ac.uk>

sbland force-pushed the 464-implement-frontend-javascript-testing branch from 053c4be to f7a61a7 Compare March 2, 2026 16:39

AdrianDAlessandro reviewed Mar 9, 2026

View reviewed changes

Conversation

sbland commented Oct 29, 2025 • edited by AdrianDAlessandro Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Key checklist

Further checks

Uh oh!

sbland commented Oct 29, 2025

Uh oh!

codecov bot commented Oct 29, 2025

Codecov Report

Uh oh!

sbland commented Oct 29, 2025

Uh oh!

Uh oh!

github-actions bot Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

sbland commented Nov 27, 2025

Uh oh!

AdrianDAlessandro left a comment

Choose a reason for hiding this comment

Uh oh!

sbland commented Mar 2, 2026

Uh oh!

AdrianDAlessandro left a comment

Choose a reason for hiding this comment

Uh oh!

AdrianDAlessandro Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

sbland Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

AdrianDAlessandro Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

sbland Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

AdrianDAlessandro Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

AdrianDAlessandro Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

sbland Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

sbland commented Mar 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sbland commented Oct 29, 2025 •

edited by AdrianDAlessandro

Loading