test: add a javascript unit test harness for the classic frontend - #623
Open
oc-tmueller wants to merge 1 commit into
Open
test: add a javascript unit test harness for the classic frontend#623oc-tmueller wants to merge 1 commit into
oc-tmueller wants to merge 1 commit into
Conversation
The scripts in js/ had no automated coverage at all, and CI had no JavaScript job. This adds the setup the other ownCloud apps use: karma with jasmine, specs in tests/js/, a test-js make target and the shared js-unit reusable workflow. The classic frontend expects the globals the server puts on the page, so the karma config loads jQuery, jQuery UI and OC from the surrounding core checkout the same way the server does, and the scripts under test run unmodified. Only js/documents.js is loaded - the other scripts in js/ are never on the same page and share one global namespace. The first specs cover where the browser may navigate to when the editor is closed and which origin post messages are exchanged with, the counterpart to the DocumentController tests. Node dependencies are installed with pnpm 9, pnpm 10 refuses the easygettext tarball in the lockfile for a missing integrity hash. The unused npm and bower rules the target replaces were dead, none of their variables were ever defined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
Contributor
|
https://github.com/owncloud/richdocuments/actions/runs/34483941568/job/102893396948?pr=623 There are a few warnings, but it passes - good. |
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
2 participants
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.
Summary
The scripts in
js/had no automated coverage and CI had no JavaScript job atall, so changes to the classic frontend could only be verified by hand. This
adds the setup the other ownCloud apps use - karma with jasmine, specs in
tests/js/, amake test-jstarget, and the sharedjs-unitreusableworkflow.
tests/js/karma.config.cjsloads jQuery, jQuery UI andOCfrom thesurrounding core checkout the same way the server loads them, so the scripts
under test run unmodified. Only
js/documents.jsis loaded: the other scriptsin
js/are never on the same page and share one global namespace, which isalso why core's own karma config lists files explicitly instead of globbing.
The file is
.cjsbecausepackage.jsondeclares"type": "module".karma-jasmine-sinonis needed because core'sspecHelper.jsinstalls aglobal
beforeEachthat usessinon.fakeServer. That also means no spec canreach the network, and each spec gets a
#testAreadiv for fixtures.pluginsare listed explicitly. Karma's defaultkarma-*discovery onlyscans the directory karma itself sits in, which finds nothing in a pnpm
layout.
test-jstarget replaces the previous npm and bower rules in theMakefile. Those were dead: none ofnodejs_deps,bower_deps,BOWERorJSDOCwas ever defined, so the rule applied to an empty target.make distoutput is unchanged.
npx.pnpm-lock.yamlhas nointegrity hash for the
easygettexttarball, which pnpm 10 refuses withERR_PNPM_MISSING_TARBALL_INTEGRITY. Override withPNPM=<command>to use alocal pnpm.
Testing
37 specs in
tests/js/documentsSpec.js, covering the frontend half of #620 -which had to be verified manually at the time, for lack of this harness:
_absoluteHttpUrl()refuses everything that is not an absolute http(s) URL,and accepts a subdirectory install and a non-default port.
_wopiOrigin()derives the origin from an absolute and from aserver-relative discovery
urlsrc, and has no origin at all for an empty ornon-http(s) one.
WOPIPostMessage()posts to that origin rather than to any origin, and postsnothing when the origin is unknown.
onStartup()takes the return URL from the hidden input the server rendersand ignores a
serverURL parameter.onClose()does not navigate to a return URL that is not an absolute http(s)URL.
origin.
Run locally against ownCloud 11 core with
make test-js, 37 specs green. Everyguard the specs describe was checked by reverting it and confirming the specs
fail.
Two things this deliberately does not do, each its own change: the Vue connector
in
src/still has no unit tests - it needs a separate vitest setup likeowncloud/web-extensionsuses - andpnpm lintremains broken and still doesnot cover
js/.🤖 Generated with Claude Code