From 4c6b6ac0fea3fadbd1ac7ccb65d74402ca54b558 Mon Sep 17 00:00:00 2001 From: jamesread Date: Thu, 10 Sep 2026 16:17:52 +0100 Subject: [PATCH 1/6] chore(lint): enforce JavaScript checks consistently Co-authored-by: Cursor --- frontend/Makefile | 13 +++---- frontend/package.json | 2 ++ frontend/vite.config.mjs | 24 ++++++------- integration-tests/.eslintrc.yml | 8 ----- integration-tests/Makefile | 12 +++++-- integration-tests/eslint.config.mjs | 31 ++++++++++++++++ integration-tests/lib/elements.js | 2 +- integration-tests/package-lock.json | 36 +++++++++++++++++++ integration-tests/package.json | 6 +++- .../scripts/find-flakey-tests-inf.mjs | 2 +- .../argumentActionFlash.mjs | 2 +- .../argumentIdCollision.mjs | 2 +- .../authRequireGuestsToLogin.mjs | 3 +- integration-tests/tests/checkbox/checkbox.mjs | 2 +- .../tests/checklist/checklist.mjs | 2 +- .../tests/confirmation/confirmation.mjs | 2 +- .../dashboardsWithBasicFieldsets.mjs | 2 +- .../emptyDashboardsAreHidden.mjs | 1 - .../tests/hiddenFooter/hiddenFooter.mjs | 4 +-- .../tests/hiddenNav/hiddenNav.mjs | 7 ++-- .../tests/justification/justification.mjs | 2 +- .../tests/localAuth/localAuth.mjs | 11 +++--- .../tests/logPersistence/logPersistence.mjs | 3 +- .../multipleDropdowns/multipleDropdowns.mjs | 2 +- .../tests/oauthLoginGithub/githubOAuth.mjs | 23 +++++------- .../tests/onlyDashboards/onlyDashboards.mjs | 3 +- integration-tests/tests/sleep/sleep.mjs | 4 +-- .../stdoutMostRecentExecution.mjs | 9 +++-- 28 files changed, 136 insertions(+), 84 deletions(-) delete mode 100644 integration-tests/.eslintrc.yml create mode 100644 integration-tests/eslint.config.mjs diff --git a/frontend/Makefile b/frontend/Makefile index b0500973..f33ac4c7 100644 --- a/frontend/Makefile +++ b/frontend/Makefile @@ -2,10 +2,11 @@ define delete-files python3 -c "import shutil;shutil.rmtree('$(1)', ignore_errors=True)" endef -codestyle: - npm install - npx eslint --fix main.js js/* resources/vue - npx stylelint style.css +codestyle: deps + npm run lint + +codestyle-fix: deps + npm run lint:fix unittests: deps npm test @@ -14,11 +15,11 @@ clean: $(call delete-files,dist) deps: - npm install + npm ci build: npx vite build dist: deps clean build -.PHONY: codestyle unittests +.PHONY: codestyle codestyle-fix unittests deps build dist clean diff --git a/frontend/package.json b/frontend/package.json index f57e7899..7580b688 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -11,6 +11,8 @@ "stylelint-config-standard": "^40.0.0" }, "scripts": { + "lint": "eslint main.js js/* resources/vue vite.config.mjs && stylelint \"style.css\" \"themes/**/*.css\"", + "lint:fix": "eslint --fix main.js js/* resources/vue vite.config.mjs && stylelint --fix \"style.css\" \"themes/**/*.css\"", "test": "node --test resources/vue/components/*.test.mjs resources/vue/utils/*.test.mjs resources/vue/stores/*.test.mjs" }, "author": "", diff --git a/frontend/vite.config.mjs b/frontend/vite.config.mjs index fc3ed2d7..fd37d068 100644 --- a/frontend/vite.config.mjs +++ b/frontend/vite.config.mjs @@ -4,16 +4,16 @@ import Components from 'unplugin-vue-components/vite' export default defineConfig({ resolve: { - dedupe: ['vue', 'vue-router'], + dedupe: ['vue', 'vue-router'] }, plugins: [ Components({ dirs: ['resources/vue/'], extensions: ['vue'], deep: true, - dts: false, + dts: false }), - vue(), + vue() ], build: { rolldownOptions: { @@ -22,25 +22,25 @@ export default defineConfig({ return } defaultHandler(level, log) - }, - }, + } + } }, server: { proxy: { '/api': { target: 'http://localhost:1337', changeOrigin: true, - secure: false, + secure: false }, '/theme.css': { target: 'http://localhost:1337', changeOrigin: true, - secure: false, + secure: false }, - "/custom-webui": { - target: "http://localhost:1337", - changeOrigin: true, + '/custom-webui': { + target: 'http://localhost:1337', + changeOrigin: true } - }, - }, + } + } }) diff --git a/integration-tests/.eslintrc.yml b/integration-tests/.eslintrc.yml deleted file mode 100644 index 8f29b855..00000000 --- a/integration-tests/.eslintrc.yml +++ /dev/null @@ -1,8 +0,0 @@ -env: - browser: true - es2021: true -extends: 'eslint:recommended' -parserOptions: - ecmaVersion: 12 - sourceType: module -rules: {} diff --git a/integration-tests/Makefile b/integration-tests/Makefile index ed93437d..1f8c41a5 100644 --- a/integration-tests/Makefile +++ b/integration-tests/Makefile @@ -1,7 +1,13 @@ default: test-install prep test-run test-install: - npm install --no-fund + npm ci --no-fund + +codestyle: test-install + npm run lint + +codestyle-fix: test-install + npm run lint:fix prep: ifneq ($(SKIP_WEBUI),1) @@ -11,7 +17,7 @@ endif test-run: # GitHub Actions fails badly on the default timeout of 2000ms - npx mocha tests --recursive -t 10000 + npm test find-flakey-tests: echo "Running test-run infinately" @@ -30,4 +36,4 @@ getsnapshot: rm -rf /opt/OliveTin-snapshot/* gh run download -D /opt/OliveTin-snapshot/ -.PHONY: default find-flakey-tests find-flakey-tests-inf prep +.PHONY: default test-install codestyle codestyle-fix test-run find-flakey-tests find-flakey-tests-inf prep diff --git a/integration-tests/eslint.config.mjs b/integration-tests/eslint.config.mjs new file mode 100644 index 00000000..f6eda8ae --- /dev/null +++ b/integration-tests/eslint.config.mjs @@ -0,0 +1,31 @@ +import js from '@eslint/js' +import globals from 'globals' + +export default [ + { + ignores: [ + 'node_modules/**', + 'tests/customJs/custom-webui/**' + ] + }, + js.configs.recommended, + { + files: ['**/*.{js,mjs}'], + languageOptions: { + ecmaVersion: 'latest', + sourceType: 'module', + globals: { + ...globals.browser, + ...globals.node, + after: 'readonly', + afterEach: 'readonly', + before: 'readonly', + beforeEach: 'readonly', + describe: 'readonly', + it: 'readonly', + runner: 'readonly', + webdriver: 'readonly' + } + } + } +] diff --git a/integration-tests/lib/elements.js b/integration-tests/lib/elements.js index abfedafc..acc3d14c 100644 --- a/integration-tests/lib/elements.js +++ b/integration-tests/lib/elements.js @@ -114,7 +114,7 @@ export function takeScreenshot (webdriver, title) { fs.mkdirSync('screenshots', { recursive: true }); title = title.replaceAll('config: ', '') - title = title.replaceAll(/[\(\)\|\*\<\>\:]/g, "_") + title = title.replaceAll(/[()|*<>:]/g, '_') title = title + '.failed-test' fs.writeFileSync('screenshots/' + title + '.png', img, 'base64') diff --git a/integration-tests/package-lock.json b/integration-tests/package-lock.json index ecab8f4c..95f49347 100644 --- a/integration-tests/package-lock.json +++ b/integration-tests/package-lock.json @@ -12,8 +12,10 @@ "wait-on": "^9.1.0" }, "devDependencies": { + "@eslint/js": "^10.0.1", "chai": "^6.2.2", "eslint": "^10.10.0", + "globals": "^17.3.0", "mocha": "^12.0.0", "selenium-webdriver": "^4.49.0" } @@ -141,6 +143,27 @@ "node": "^20.19.0 || ^22.13.0 || >=24" } }, + "node_modules/@eslint/js": { + "version": "10.0.1", + "resolved": "https://registry.npmjs.org/@eslint/js/-/js-10.0.1.tgz", + "integrity": "sha512-zeR9k5pd4gxjZ0abRoIaxdc7I3nDktoXZk2qOv9gCNWx3mVwEn32VRhyLaRsDiJjTs0xq/T8mfPtyuXu7GWBcA==", + "dev": true, + "license": "MIT", + "engines": { + "node": "^20.19.0 || ^22.13.0 || >=24" + }, + "funding": { + "url": "https://eslint.org/donate" + }, + "peerDependencies": { + "eslint": "^10.0.0" + }, + "peerDependenciesMeta": { + "eslint": { + "optional": true + } + } + }, "node_modules/@eslint/object-schema": { "version": "3.0.5", "resolved": "https://registry.npmjs.org/@eslint/object-schema/-/object-schema-3.0.5.tgz", @@ -966,6 +989,19 @@ "node": ">=10.13.0" } }, + "node_modules/globals": { + "version": "17.12.0", + "resolved": "https://registry.npmjs.org/globals/-/globals-17.12.0.tgz", + "integrity": "sha512-cezEd/DTyyht9cvSSURyygXPfy04GtWO/5e6ZPvH7fCtjKz9PYOmuawphw1Ctd1f6C+5JypXfGD7ahNMXvevBA==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/gopd": { "version": "1.2.0", "resolved": "https://registry.npmjs.org/gopd/-/gopd-1.2.0.tgz", diff --git a/integration-tests/package.json b/integration-tests/package.json index 5065633a..c3f9dfd6 100644 --- a/integration-tests/package.json +++ b/integration-tests/package.json @@ -6,13 +6,17 @@ "main": "index.js", "type": "module", "scripts": { - "test": "echo \"Error: no test specified\" && exit 1" + "lint": "eslint tests lib scripts runner.mjs mochaSetup.mjs", + "lint:fix": "npm run lint -- --fix", + "test": "mocha tests --recursive -t 10000" }, "author": "", "license": "AGPL-3.0-only", "devDependencies": { + "@eslint/js": "^10.0.1", "chai": "^6.2.2", "eslint": "^10.10.0", + "globals": "^17.3.0", "mocha": "^12.0.0", "selenium-webdriver": "^4.49.0" }, diff --git a/integration-tests/scripts/find-flakey-tests-inf.mjs b/integration-tests/scripts/find-flakey-tests-inf.mjs index b50cb322..c7d58800 100644 --- a/integration-tests/scripts/find-flakey-tests-inf.mjs +++ b/integration-tests/scripts/find-flakey-tests-inf.mjs @@ -104,7 +104,7 @@ function runMochaOnce () { try { report = JSON.parse(readFileSync(reportPath, 'utf8')) } catch { - report = null + // Keep the default null report when Mocha did not produce valid JSON. } try { diff --git a/integration-tests/tests/argumentActionFlash/argumentActionFlash.mjs b/integration-tests/tests/argumentActionFlash/argumentActionFlash.mjs index a4b423fa..0dc3e195 100644 --- a/integration-tests/tests/argumentActionFlash/argumentActionFlash.mjs +++ b/integration-tests/tests/argumentActionFlash/argumentActionFlash.mjs @@ -28,7 +28,7 @@ async function waitForActionSuccessFlash (actionTitle) { const button = await getActionButton(webdriver, actionTitle) const classAttr = await button.getAttribute('class') return classAttr && classAttr.includes('action-success') - } catch (e) { + } catch { return false } }), diff --git a/integration-tests/tests/argumentIdCollision/argumentIdCollision.mjs b/integration-tests/tests/argumentIdCollision/argumentIdCollision.mjs index dca6282e..e3f669fd 100644 --- a/integration-tests/tests/argumentIdCollision/argumentIdCollision.mjs +++ b/integration-tests/tests/argumentIdCollision/argumentIdCollision.mjs @@ -45,7 +45,7 @@ async function waitForTerminalOutput (expectedSubstring) { const output = await getTerminalBuffer() return output && output.includes(expectedSubstring) - } catch (e) { + } catch { return false } }), diff --git a/integration-tests/tests/authRequireGuestsToLogin/authRequireGuestsToLogin.mjs b/integration-tests/tests/authRequireGuestsToLogin/authRequireGuestsToLogin.mjs index 131f1d73..b7363795 100644 --- a/integration-tests/tests/authRequireGuestsToLogin/authRequireGuestsToLogin.mjs +++ b/integration-tests/tests/authRequireGuestsToLogin/authRequireGuestsToLogin.mjs @@ -1,8 +1,7 @@ import { describe, it, before, after } from 'mocha' import { expect } from 'chai' -import { By, until } from 'selenium-webdriver' +import { By } from 'selenium-webdriver' import { - getRootAndWait, takeScreenshotOnFailure, } from '../../lib/elements.js' diff --git a/integration-tests/tests/checkbox/checkbox.mjs b/integration-tests/tests/checkbox/checkbox.mjs index 4b25dffd..b8ad3ff1 100644 --- a/integration-tests/tests/checkbox/checkbox.mjs +++ b/integration-tests/tests/checkbox/checkbox.mjs @@ -47,7 +47,7 @@ async function waitForTerminalOutput(expectedValue) { } return output.trim().includes(`Checkbox value: ${expectedValue}`) - } catch (e) { + } catch { return false } }), diff --git a/integration-tests/tests/checklist/checklist.mjs b/integration-tests/tests/checklist/checklist.mjs index 8fd18e9a..bb3f03d4 100644 --- a/integration-tests/tests/checklist/checklist.mjs +++ b/integration-tests/tests/checklist/checklist.mjs @@ -44,7 +44,7 @@ async function pollTerminal(matcher, timeoutMs = DEFAULT_UI_WAIT_MS) { } return matcher(output.trim()) - } catch (e) { + } catch { return false } }), diff --git a/integration-tests/tests/confirmation/confirmation.mjs b/integration-tests/tests/confirmation/confirmation.mjs index 6f56dd3c..5bef82e5 100644 --- a/integration-tests/tests/confirmation/confirmation.mjs +++ b/integration-tests/tests/confirmation/confirmation.mjs @@ -59,7 +59,7 @@ async function waitForTerminalOutput (expectedSubstring) { const output = await getTerminalBuffer() return output && output.includes(expectedSubstring) - } catch (e) { + } catch { return false } }), diff --git a/integration-tests/tests/dashboardsWithBasicFieldsets/dashboardsWithBasicFieldsets.mjs b/integration-tests/tests/dashboardsWithBasicFieldsets/dashboardsWithBasicFieldsets.mjs index a4f343bd..a8f2e357 100644 --- a/integration-tests/tests/dashboardsWithBasicFieldsets/dashboardsWithBasicFieldsets.mjs +++ b/integration-tests/tests/dashboardsWithBasicFieldsets/dashboardsWithBasicFieldsets.mjs @@ -1,6 +1,6 @@ import { describe, it, before, after } from 'mocha' import { expect, assert } from 'chai' -import { By, until, Condition } from 'selenium-webdriver' +import { By } from 'selenium-webdriver' //import * as waitOn from 'wait-on' import { getRootAndWait, diff --git a/integration-tests/tests/emptyDashboardsAreHidden/emptyDashboardsAreHidden.mjs b/integration-tests/tests/emptyDashboardsAreHidden/emptyDashboardsAreHidden.mjs index 035f5428..73e14db3 100644 --- a/integration-tests/tests/emptyDashboardsAreHidden/emptyDashboardsAreHidden.mjs +++ b/integration-tests/tests/emptyDashboardsAreHidden/emptyDashboardsAreHidden.mjs @@ -1,6 +1,5 @@ import { describe, it, before, after } from 'mocha' import { expect } from 'chai' -import { By, until, Condition } from 'selenium-webdriver' //import * as waitOn from 'wait-on' import { getRootAndWait, diff --git a/integration-tests/tests/hiddenFooter/hiddenFooter.mjs b/integration-tests/tests/hiddenFooter/hiddenFooter.mjs index 8ff192ef..330db5aa 100644 --- a/integration-tests/tests/hiddenFooter/hiddenFooter.mjs +++ b/integration-tests/tests/hiddenFooter/hiddenFooter.mjs @@ -2,9 +2,7 @@ import { describe, it, before, after } from 'mocha' import { expect } from 'chai' import { By } from 'selenium-webdriver' -import { - getRootAndWait, - getActionButtons, +import { takeScreenshotOnFailure, } from '../../lib/elements.js' diff --git a/integration-tests/tests/hiddenNav/hiddenNav.mjs b/integration-tests/tests/hiddenNav/hiddenNav.mjs index 147df853..9fff5316 100644 --- a/integration-tests/tests/hiddenNav/hiddenNav.mjs +++ b/integration-tests/tests/hiddenNav/hiddenNav.mjs @@ -1,8 +1,5 @@ -import { expect } from 'chai' -import { By } from 'selenium-webdriver' -import { - getRootAndWait, - getActionButtons, +import { + getRootAndWait, takeScreenshotOnFailure, } from '../../lib/elements.js' diff --git a/integration-tests/tests/justification/justification.mjs b/integration-tests/tests/justification/justification.mjs index 23b1ee19..0efd4ff4 100644 --- a/integration-tests/tests/justification/justification.mjs +++ b/integration-tests/tests/justification/justification.mjs @@ -88,7 +88,7 @@ async function waitForTerminalOutput (expectedSubstring) { const output = await getTerminalBuffer() return output && output.includes(expectedSubstring) - } catch (e) { + } catch { return false } }), diff --git a/integration-tests/tests/localAuth/localAuth.mjs b/integration-tests/tests/localAuth/localAuth.mjs index 163a385e..70fe3f2d 100644 --- a/integration-tests/tests/localAuth/localAuth.mjs +++ b/integration-tests/tests/localAuth/localAuth.mjs @@ -1,8 +1,7 @@ import { describe, it, before, after } from 'mocha' import { expect } from 'chai' -import { By, until, Condition } from 'selenium-webdriver' +import { By, until } from 'selenium-webdriver' import { - getRootAndWait, takeScreenshotOnFailure, } from '../../lib/elements.js' @@ -47,7 +46,7 @@ describe('config: localAuth', function () { // Check if any login-related elements are present const bodyText = await webdriver.findElement(By.tagName('body')).getText() console.log('Login page content:', bodyText.substring(0, 300)) - + // For now, just verify we can navigate to the login page // The page content rendering is a separate frontend issue console.log('Login page navigation successful') @@ -67,11 +66,11 @@ describe('config: localAuth', function () { if (usernameFields.length > 0 && passwordFields.length > 0 && loginButtons.length > 0) { console.log('Login form found, attempting login') - + // Fill in credentials await usernameFields[0].clear() await usernameFields[0].sendKeys('testuser') - + await passwordFields[0].clear() await passwordFields[0].sendKeys('testpass123') @@ -100,4 +99,4 @@ describe('config: localAuth', function () { console.log('Login form not found - skipping login test') } }) -}) \ No newline at end of file +}) diff --git a/integration-tests/tests/logPersistence/logPersistence.mjs b/integration-tests/tests/logPersistence/logPersistence.mjs index f7d93bd7..09fa9e51 100644 --- a/integration-tests/tests/logPersistence/logPersistence.mjs +++ b/integration-tests/tests/logPersistence/logPersistence.mjs @@ -5,7 +5,6 @@ import fs from 'fs' import path from 'path' import { getRootAndWait, - getActionButtons, takeScreenshotOnFailure, } from '../../lib/elements.js' @@ -210,7 +209,7 @@ describe('config: logPersistence', function () { const text = await body.getText() // The log should contain the output from the echo command return text.includes('Hello from persisted log test') || text.includes(firstExecutionTrackingId) - } catch (e) { + } catch { return false } }), diff --git a/integration-tests/tests/multipleDropdowns/multipleDropdowns.mjs b/integration-tests/tests/multipleDropdowns/multipleDropdowns.mjs index 90cb16f6..966f3d9d 100644 --- a/integration-tests/tests/multipleDropdowns/multipleDropdowns.mjs +++ b/integration-tests/tests/multipleDropdowns/multipleDropdowns.mjs @@ -1,6 +1,6 @@ import { describe, it, before, after } from 'mocha' import { expect } from 'chai' -import { By, until, Condition, Key } from 'selenium-webdriver' +import { By, Condition, Key } from 'selenium-webdriver' import { getRootAndWait, getActionButtons, diff --git a/integration-tests/tests/oauthLoginGithub/githubOAuth.mjs b/integration-tests/tests/oauthLoginGithub/githubOAuth.mjs index 458b6487..3a7d7ad3 100644 --- a/integration-tests/tests/oauthLoginGithub/githubOAuth.mjs +++ b/integration-tests/tests/oauthLoginGithub/githubOAuth.mjs @@ -1,8 +1,7 @@ import { describe, it, before, after } from 'mocha' import { expect } from 'chai' -import { By, until, Condition } from 'selenium-webdriver' +import { By, until } from 'selenium-webdriver' import { - getRootAndWait, takeScreenshotOnFailure, } from '../../lib/elements.js' @@ -56,14 +55,14 @@ describe('config: githubOAuth', function () { // Button may show "Login with GitHub" or "Login with undefined" depending on provider.name vs provider.title // We'll check for the presence of the button and verify it's in the OAuth section expect(githubButtons.length).to.be.greaterThan(0, 'At least one OAuth button should be present') - + // The first button should be GitHub since it's the only provider in the config const githubButton = githubButtons[0] const buttonText = await githubButton.getText() - + // Button should contain "Login with" and the provider should be configured as GitHub expect(buttonText).to.include('Login with', 'Button should have "Login with" prefix') - + console.log('GitHub OAuth button found with text:', buttonText) }) @@ -78,16 +77,14 @@ describe('config: githubOAuth', function () { // Since the test config only has one provider (GitHub), we can use the first button const githubButtons = await webdriver.findElements(By.css('.oauth-button')) expect(githubButtons.length).to.be.greaterThan(0, 'At least one OAuth button should be present') - + const githubButton = githubButtons[0] const buttonText = await githubButton.getText() console.log('Button text:', buttonText) - + // Verify it's the GitHub button (should contain "github" in the text) expect(buttonText.toLowerCase()).to.include('github', 'Button should be GitHub OAuth button') - // Check for provider icon (if present) - const providerIcons = await githubButton.findElements(By.css('.provider-icon')) const providerNames = await githubButton.findElements(By.css('.provider-name')) // Provider name may show "GitHub" (from title) or be undefined (if using name field) // Just verify the structure is present @@ -111,11 +108,8 @@ describe('config: githubOAuth', function () { // Find GitHub OAuth button (should be the first/only one in our test config) const githubButtons = await webdriver.findElements(By.css('.oauth-button')) expect(githubButtons.length).to.be.greaterThan(0, 'OAuth button should be present') - - const githubButton = githubButtons[0] - // Get the current URL before clicking - const initialUrl = await webdriver.getCurrentUrl() + const githubButton = githubButtons[0] // Click the button await githubButton.click() @@ -124,7 +118,7 @@ describe('config: githubOAuth', function () { // Since we can't actually complete OAuth flow, we check that the button // click handler is set up correctly by verifying the button exists and is clickable // In a real scenario, this would redirect to GitHub's OAuth page - + // Give a small delay to allow any navigation to start await new Promise(resolve => setTimeout(resolve, 1000)) @@ -133,4 +127,3 @@ describe('config: githubOAuth', function () { console.log('GitHub OAuth button click verified (redirect would happen in production)') }) }) - diff --git a/integration-tests/tests/onlyDashboards/onlyDashboards.mjs b/integration-tests/tests/onlyDashboards/onlyDashboards.mjs index fd4b6b72..42050e9e 100644 --- a/integration-tests/tests/onlyDashboards/onlyDashboards.mjs +++ b/integration-tests/tests/onlyDashboards/onlyDashboards.mjs @@ -6,7 +6,6 @@ import { getActionButtons, getNavigationLinks, openSidebar, - closeSidebar, takeScreenshotOnFailure, } from '../../lib/elements.js' @@ -43,7 +42,7 @@ describe('config: onlyDashboards', function () { const firstDashboardLink = await webdriver.findElement(By.css('li[title="My Dashboard"]'), 'The first dashboard link should be present') assert.isNotNull(firstDashboardLink, 'First dashboard link should not be null') assert.isTrue(await firstDashboardLink.isDisplayed(), 'First dashboard link should be displayed') - + const actionButtonsOnDashboard = await getActionButtons() assert.isArray(actionButtonsOnDashboard, 'Action buttons on dashboard should be an array') assert.lengthOf(actionButtonsOnDashboard, 3, 'Action buttons on dashboard should have 3 buttons') diff --git a/integration-tests/tests/sleep/sleep.mjs b/integration-tests/tests/sleep/sleep.mjs index d287794e..e8b59734 100644 --- a/integration-tests/tests/sleep/sleep.mjs +++ b/integration-tests/tests/sleep/sleep.mjs @@ -1,9 +1,7 @@ -import * as process from 'node:process' import { describe, it, before, after } from 'mocha' import { expect } from 'chai' -import { By, Condition } from 'selenium-webdriver' +import { By } from 'selenium-webdriver' import { - takeScreenshot, takeScreenshotOnFailure, findExecutionDialog, requireExecutionDialogStatus, diff --git a/integration-tests/tests/stdoutMostRecentExecution/stdoutMostRecentExecution.mjs b/integration-tests/tests/stdoutMostRecentExecution/stdoutMostRecentExecution.mjs index 5388eb01..97cc2831 100644 --- a/integration-tests/tests/stdoutMostRecentExecution/stdoutMostRecentExecution.mjs +++ b/integration-tests/tests/stdoutMostRecentExecution/stdoutMostRecentExecution.mjs @@ -3,7 +3,6 @@ import { expect } from 'chai' import { By, Condition } from 'selenium-webdriver' import { getRootAndWait, - getActionButtons, takeScreenshotOnFailure, } from '../../lib/elements.js' @@ -116,13 +115,13 @@ describe('config: stdout-most-recent-execution', function () { // Output should change from initial state and contain actual output // (not "Waiting...", "No execution found", or the same as initialText) const hasChanged = newText !== initialText - const hasValidOutput = newText && - !newText.includes('Waiting...') && - !newText.includes('No execution found') && + const hasValidOutput = newText && + !newText.includes('Waiting...') && + !newText.includes('No execution found') && !newText.includes('Error:') && newText.trim().length > 0 return hasChanged && hasValidOutput - } catch (e) { + } catch { return false } }), From 98b6b9d0d28605d7bc1f0344022df13064bfaade Mon Sep 17 00:00:00 2001 From: jamesread Date: Thu, 10 Sep 2026 16:18:30 +0100 Subject: [PATCH 2/6] ci: run deterministic JavaScript checks on pull requests Co-authored-by: Cursor --- .github/workflows/codestyle.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/codestyle.yml b/.github/workflows/codestyle.yml index 7009314a..691da336 100644 --- a/.github/workflows/codestyle.yml +++ b/.github/workflows/codestyle.yml @@ -5,6 +5,17 @@ on: push: paths: - '.github/workflows/codestyle.yml' + - 'Makefile' + - 'frontend/**' + - 'integration-tests/**' + - 'proto/**' + - 'service/**' + pull_request: + branches: + - next + paths: + - '.github/workflows/codestyle.yml' + - 'Makefile' - 'frontend/**' - 'integration-tests/**' - 'proto/**' @@ -35,9 +46,16 @@ jobs: uses: actions/setup-node@v4 with: node-version: '22' + cache: 'npm' + cache-dependency-path: | + frontend/package-lock.json + integration-tests/package-lock.json - name: frontend run: make -wC frontend codestyle - name: frontend unit tests run: make -wC frontend unittests + + - name: integration tests codestyle + run: make -wC integration-tests codestyle From bbff2f4ba1b50ae463f3eeee133ed051983fce5d Mon Sep 17 00:00:00 2001 From: jamesread Date: Thu, 10 Sep 2026 16:20:28 +0100 Subject: [PATCH 3/6] fix: synchronize watcher and execution log state Co-authored-by: Cursor --- service/internal/executor/executor.go | 32 +++++++++++----- service/internal/executor/executor_test.go | 22 +++++++++++ .../internal/filehelper/file_change_notify.go | 8 +++- .../filehelper/file_change_notify_test.go | 37 +++++++++++++++++++ 4 files changed, 88 insertions(+), 11 deletions(-) create mode 100644 service/internal/filehelper/file_change_notify_test.go diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index 8bdec7ca..8cd4c8f7 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -153,6 +153,18 @@ type InternalLogEntry struct { TimedOut bool } +func cloneInternalLogEntry(entry *InternalLogEntry) *InternalLogEntry { + if entry == nil { + return nil + } + + cloned := *entry + cloned.Arguments = maps.Clone(entry.Arguments) + cloned.Tags = slices.Clone(entry.Tags) + + return &cloned +} + // .Binding can be nil, so we need to handle that. func (e *InternalLogEntry) GetBindingId() string { if e.Binding == nil { @@ -273,7 +285,7 @@ func (e *Executor) GetLogTrackingIds(startOffset int64, pageCount int64) ([]*Int if totalLogCount > 0 { for i := startIndex; i >= endIndex; i-- { - trackingIds = append(trackingIds, e.logs[e.logsTrackingIdsByDate[i]]) + trackingIds = append(trackingIds, cloneInternalLogEntry(e.logs[e.logsTrackingIdsByDate[i]])) } } @@ -303,7 +315,7 @@ func (e *Executor) filterLogsByACL(cfg *config.Config, user *authpublic.Authenti entry := e.logs[trackingId] if shouldIncludeLogEntry(cfg, user, entry, filterDate, hasDateFilter) { - filtered = append(filtered, entry) + filtered = append(filtered, cloneInternalLogEntry(entry)) } } @@ -399,26 +411,28 @@ func (e *Executor) GetLogTrackingIdsACL(cfg *config.Config, user *authpublic.Aut func (e *Executor) GetLog(trackingID string) (*InternalLogEntry, bool) { e.logmutex.RLock() + defer e.logmutex.RUnlock() entry, found := e.logs[trackingID] - - e.logmutex.RUnlock() - - return entry, found + return cloneInternalLogEntry(entry), found } func (e *Executor) GetLogsByBindingId(bindingId string) []*InternalLogEntry { e.logmutex.RLock() + defer e.logmutex.RUnlock() logs, found := e.LogsByBindingId[bindingId] - e.logmutex.RUnlock() - if !found { return make([]*InternalLogEntry, 0) } - return logs + cloned := make([]*InternalLogEntry, 0, len(logs)) + for _, entry := range logs { + cloned = append(cloned, cloneInternalLogEntry(entry)) + } + + return cloned } // shouldCountExecution checks if a log entry should be counted for rate limiting. diff --git a/service/internal/executor/executor_test.go b/service/internal/executor/executor_test.go index 2261a172..f7c23002 100644 --- a/service/internal/executor/executor_test.go +++ b/service/internal/executor/executor_test.go @@ -38,6 +38,28 @@ func testingExecutor() (*Executor, *config.Config) { return e, cfg } +func TestGetLogReturnsDefensiveCopy(t *testing.T) { + e := DefaultExecutor(config.DefaultConfig()) + e.logs["tracking-id"] = &InternalLogEntry{ + Arguments: map[string]string{"message": "original"}, + Output: "original", + Tags: []string{"original"}, + } + + entry, found := e.GetLog("tracking-id") + require.True(t, found) + + entry.Arguments["message"] = "changed" + entry.Output = "changed" + entry.Tags[0] = "changed" + + stored, found := e.GetLog("tracking-id") + require.True(t, found) + assert.Equal(t, "original", stored.Arguments["message"]) + assert.Equal(t, "original", stored.Output) + assert.Equal(t, []string{"original"}, stored.Tags) +} + func TestCreateExecutorAndExec(t *testing.T) { e, cfg := testingExecutor() diff --git a/service/internal/filehelper/file_change_notify.go b/service/internal/filehelper/file_change_notify.go index a6aec552..5f38536a 100644 --- a/service/internal/filehelper/file_change_notify.go +++ b/service/internal/filehelper/file_change_notify.go @@ -273,13 +273,17 @@ func processDebounce(ctx *watchContext) { if logEntry.callbackComplete || logEntry.callbackWrapper == nil { log.Debugf("fsnotify event callback queued within debounce delay: %v", ctx.filename) + callback := ctx.callback + eventName := ctx.event.Name logEntry.callbackComplete = false logEntry.callbackWrapper = time.AfterFunc(debounceDelay, func() { - log.Debugf("fsnotify event callback being fired: %v", ctx.filename) + log.Debugf("fsnotify event callback being fired: %v", eventName) - ctx.callback(ctx.event.Name) + callback(eventName) + debounceWriteLogMutex.Lock() logEntry.callbackComplete = true + debounceWriteLogMutex.Unlock() }) } else { log.Debugf("fsnotify event suppressed because it's within the debounce delay: %v", ctx.filename) diff --git a/service/internal/filehelper/file_change_notify_test.go b/service/internal/filehelper/file_change_notify_test.go new file mode 100644 index 00000000..48c1d88d --- /dev/null +++ b/service/internal/filehelper/file_change_notify_test.go @@ -0,0 +1,37 @@ +package filehelper + +import ( + "testing" + "time" + + "github.com/fsnotify/fsnotify" + "github.com/stretchr/testify/require" +) + +func TestProcessDebounceCapturesEventName(t *testing.T) { + debounceWriteLogMutex.Lock() + debounceWriteLog = make(map[string]*FsNotifyLogEntry) + debounceWriteLogMutex.Unlock() + + callbackNames := make(chan string, 1) + firstEvent := fsnotify.Event{Name: "first"} + ctx := &watchContext{ + callback: func(filename string) { + callbackNames <- filename + }, + event: &firstEvent, + filename: t.Name(), + } + + processDebounce(ctx) + + secondEvent := fsnotify.Event{Name: "second"} + ctx.event = &secondEvent + + select { + case callbackName := <-callbackNames: + require.Equal(t, firstEvent.Name, callbackName) + case <-time.After(time.Second): + t.Fatal("debounced callback did not run") + } +} From a5f2e251e73e7ce86e9d13423dfe8852322c8752 Mon Sep 17 00:00:00 2001 From: jamesread Date: Thu, 10 Sep 2026 16:21:00 +0100 Subject: [PATCH 4/6] test: add Go race detector coverage Co-authored-by: Cursor --- .github/workflows/race.yml | 36 ++++++++++++++++++++++++++++++++++++ service/Makefile | 5 ++++- 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/race.yml diff --git a/.github/workflows/race.yml b/.github/workflows/race.yml new file mode 100644 index 00000000..6a5be2c5 --- /dev/null +++ b/.github/workflows/race.yml @@ -0,0 +1,36 @@ +name: Go race detector + +on: + push: + branches: + - main + - next + paths: + - '.github/workflows/race.yml' + - 'service/**' + pull_request: + branches: + - next + paths: + - '.github/workflows/race.yml' + - 'service/**' + +permissions: + contents: read + +jobs: + race: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Go + uses: actions/setup-go@v5 + with: + go-version-file: 'service/go.mod' + cache: true + cache-dependency-path: 'service/go.mod' + + - name: Run race detector + run: make -wC service unittests-race diff --git a/service/Makefile b/service/Makefile index 309ec3dd..9a94cd9f 100644 --- a/service/Makefile +++ b/service/Makefile @@ -44,6 +44,9 @@ unittests: unittests-fast: go test ./... -count=1 +unittests-race: + go test -race ./... -count=1 + find-flakey-tests: echo "Running unittests-fast infinitely" sh -c "while $(MAKE) unittests-fast; do :; done" @@ -54,7 +57,7 @@ find-flakey-tests-inf: go-tools: go install "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.13.2" -.PHONY: codestyle go-tools unittests unittests-fast find-flakey-tests find-flakey-tests-inf +.PHONY: codestyle go-tools unittests unittests-fast unittests-race find-flakey-tests find-flakey-tests-inf go-tools-all: go install "github.com/bufbuild/buf/cmd/buf" From bbaaf377475563a5605951cddd380c2025b6b186 Mon Sep 17 00:00:00 2001 From: jamesread Date: Thu, 10 Sep 2026 16:22:13 +0100 Subject: [PATCH 5/6] chore(docs): expose documentation checks locally Co-authored-by: Cursor --- .github/workflows/docs-antora.yml | 2 ++ .pre-commit-config.yaml | 7 +++++++ AGENTS.md | 2 ++ CONTRIBUTING.adoc | 1 + Makefile | 6 +++++- 5 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs-antora.yml b/.github/workflows/docs-antora.yml index 27ddfa79..ee9351a2 100644 --- a/.github/workflows/docs-antora.yml +++ b/.github/workflows/docs-antora.yml @@ -3,12 +3,14 @@ on: push: paths: - 'docs/**' + - 'service/internal/config/config.go' - 'local-antora-playbook.yml' - 'local-antora-playbook-ci.yml' - '.github/workflows/docs-antora.yml' pull_request: paths: - 'docs/**' + - 'service/internal/config/config.go' - 'local-antora-playbook.yml' - 'local-antora-playbook-ci.yml' - '.github/workflows/docs-antora.yml' diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 761ca622..14d45b80 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -33,6 +33,13 @@ repos: - repo: local hooks: + - id: docs-check + name: docs-check + entry: make docs-check + language: system + pass_filenames: false + files: ^(docs/|service/internal/config/config\.go|Makefile) + - id: service-codestyle name: service-codestyle entry: make service-codestyle diff --git a/AGENTS.md b/AGENTS.md index a88cd103..35934df8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,8 @@ If you are looking for OliveTin's AI policy, you can find it in `AI.md`. - From repo root: `cd service && make unittests` - Code style (after editing code in `service/`): - From repo root: `cd service && make codestyle` (runs `go fmt` and `golangci-lint`; install linter via `make go-tools`) +- Documentation checks: + - From repo root: `make docs-check` (validates documented config key casing and local AsciiDoc anchor links) - Integration tests (Mocha + Selenium): - All tests: `make it` (from repo root; builds webui + service binary, then runs Mocha) - Single test: `cd integration-tests && npx --yes mocha tests/general/general.mjs` diff --git a/CONTRIBUTING.adoc b/CONTRIBUTING.adoc index 1e131e44..f46147d0 100644 --- a/CONTRIBUTING.adoc +++ b/CONTRIBUTING.adoc @@ -64,6 +64,7 @@ The project layout is reasonably straightforward; * See the `Makefile` for common targets. This project was originally created on top of Fedora, but it should be usable on Debian/your faveourite distro with minor changes (if any). * End-user documentation (AsciiDoc for link:https://docs.olivetin.app[docs.olivetin.app]) lives in `docs/` as an Antora component; the published site is built from the separate link:https://github.com/OliveTin/docs.olivetin.app[docs.olivetin.app] repository. +* Run `make docs-check` after changing documentation or the service configuration schema. * The API is defined in protobuf+Connect RPC - you will need to `make proto`. * The Go daemon is built from the `cmd` and `internal` directories mostly. * The webui is just a single page application with a bit of Javascript in the `webui` directory. This can happily be hosted on another webserver. diff --git a/Makefile b/Makefile index 92bf824d..3fe3b7fa 100644 --- a/Makefile +++ b/Makefile @@ -28,6 +28,10 @@ frontend-codestyle: frontend-unittests: $(MAKE) -wC frontend unittests +docs-check: + python3 docs/modules/ROOT/check_config_keys.py + python3 docs/modules/ROOT/check_chevron_links.py + it: $(MAKE) -wC integration-tests @@ -81,4 +85,4 @@ config-tool: devcheck: python3 scripts/devcheck.py $(ARGS) -.PHONY: proto default service windows-resources windows-msi frontend-unittests it devcheck +.PHONY: proto default service windows-resources windows-msi frontend-unittests docs-check it devcheck From 4c4cb7452f9c66dac0b949b99f5f9321b4af37ec Mon Sep 17 00:00:00 2001 From: jamesread Date: Thu, 10 Sep 2026 16:42:00 +0100 Subject: [PATCH 6/6] fix: deep-clone ActionBinding when copying log entries Callers mutating Binding.ID or OnDashboards no longer race with executor-owned state returned from GetLog helpers. Co-authored-by: Cursor --- service/internal/executor/executor.go | 12 ++++++++++++ service/internal/executor/executor_test.go | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/service/internal/executor/executor.go b/service/internal/executor/executor.go index 8cd4c8f7..21d23df8 100644 --- a/service/internal/executor/executor.go +++ b/service/internal/executor/executor.go @@ -153,6 +153,17 @@ type InternalLogEntry struct { TimedOut bool } +func cloneActionBinding(binding *ActionBinding) *ActionBinding { + if binding == nil { + return nil + } + + cloned := *binding + cloned.OnDashboards = slices.Clone(binding.OnDashboards) + + return &cloned +} + func cloneInternalLogEntry(entry *InternalLogEntry) *InternalLogEntry { if entry == nil { return nil @@ -161,6 +172,7 @@ func cloneInternalLogEntry(entry *InternalLogEntry) *InternalLogEntry { cloned := *entry cloned.Arguments = maps.Clone(entry.Arguments) cloned.Tags = slices.Clone(entry.Tags) + cloned.Binding = cloneActionBinding(entry.Binding) return &cloned } diff --git a/service/internal/executor/executor_test.go b/service/internal/executor/executor_test.go index f7c23002..6c992935 100644 --- a/service/internal/executor/executor_test.go +++ b/service/internal/executor/executor_test.go @@ -44,6 +44,12 @@ func TestGetLogReturnsDefensiveCopy(t *testing.T) { Arguments: map[string]string{"message": "original"}, Output: "original", Tags: []string{"original"}, + Binding: &ActionBinding{ + ID: "original-binding", + OnDashboards: []DashboardNavigationTarget{ + {Title: "original"}, + }, + }, } entry, found := e.GetLog("tracking-id") @@ -52,12 +58,17 @@ func TestGetLogReturnsDefensiveCopy(t *testing.T) { entry.Arguments["message"] = "changed" entry.Output = "changed" entry.Tags[0] = "changed" + entry.Binding.ID = "changed-binding" + entry.Binding.OnDashboards[0].Title = "changed" stored, found := e.GetLog("tracking-id") require.True(t, found) assert.Equal(t, "original", stored.Arguments["message"]) assert.Equal(t, "original", stored.Output) assert.Equal(t, []string{"original"}, stored.Tags) + require.NotNil(t, stored.Binding) + assert.Equal(t, "original-binding", stored.Binding.ID) + assert.Equal(t, []DashboardNavigationTarget{{Title: "original"}}, stored.Binding.OnDashboards) } func TestCreateExecutorAndExec(t *testing.T) {