Nbb UI tests: first cut

[borkdude]

This adds browser tests based on Nbb using playwright. It operates on the static build and browses through all notebooks and asserts that no errors are logged via js/console.errror.

[mk]

The general approach here is to browse to all notebooks in the static build and assert they don't log any console errors, correct?

Did you verify that viewer errors are logged and lead to a failure or should we still do that?

[borkdude]

The general approach here is to browse to all notebooks in the static build and assert they don't log any console errors, correct?

Correct.

Did you verify that viewer errors are logged and lead to a failure or should we still do that?

I verified this in the REPL, but we can verify this by making a test commit which should trigger this. Do you know of anything that should trigger a console error in a clerk notebook?

[borkdude]

@mk Addressed everything, except one comment about the extra dir. Imo it's better to start coding in a separate file and use bb.edn only for hooking up code to task names, but given that the code is relatively thin, I'd be ok with moving it to the bb.edn for now. Just let me know.

[borkdude]

Hmm, somehow removing package lock json causes an error, I'll look into it.

[borkdude]

Running bb test:static-app bee6aa1bde9dfe88828171b1217e1c596d402ad9 locally works fine though.

[borkdude]

@mk We solved the problem that the viewer JS is "refreshed" as needed on google cloud before anything else runs. Having that in place, I could test and confirm that logging with js/console.error makes the tests fail:

https://github.com/nextjournal/clerk/runs/5467522586?check_suite_focus=true

I consider this PR mergeable. Let me know if you agree.

[mk]

Was about to merge this until I saw that you added an explicit js/console.error in 0465c74. So I'd like to verify that the tests also catch other failure modes (sci read or eval errors, as well as runtime exceptions). I plan to do this first thing tomorrow and then ship it.

[borkdude]

I think you made a mistake with f71bcb4 since the tests contain a static build test which needs the JS asset to already be present. Triggering a rebuild will likely fail the tests.

[mk]

@borkdude you mean this one?

clerk/test/nextjournal/clerk_test.clj

Lines 17 to 27 in e031d51

	(deftest static-app
	(let [url* (volatile! nil)]
	(with-redefs [clojure.java.browse/browse-url (fn [path]
	(vreset! url* path))]
	(testing "browser receives canonical url in this system arch"
	(fs/with-temp-dir [temp {}]
	(let [expected (-> (str/join (java.io.File/separator) [(.toString temp) "index.html"])
	(str/replace (java.io.File/separator) "/"))]
	(clerk/build-static-app! {:paths ["notebooks/hello.clj"]
	:out-path temp})
	(is (= expected @url*))))))))

If yes that should not be a problem since it's not asserting anything about this. In general, I'd like to keep the fast feedback loop the clojure tests currently bring and would prefer it if we managed to keep them without a dependency on the viewer js build.

[mk]

Oh, I see what you mean now. The slurp will fail if the js hasn't been compiled yet. I guess we can pass an override setting for the tests.