Add unit tests for create-if-not-exists and misc test improvements

- Added unit test as it is used by several handlers
- Fixed test suite failing from last commit now that a test fs is used
- Fixed test stat implementation
- Extracted out Plugin schema to make usage of it less impl dependent
- Made test.helper alias consistent
- Made async testing consistent for export-test

.clj-kondo/config.edn

@@ -60,6 +60,7 @@
              frontend.search search
              frontend.state state
              frontend.template template
+             frontend.test.helper test-helper
              frontend.ui ui
              frontend.util util
              frontend.util.clock clock
@@ -79,7 +80,8 @@
              logseq.graph-parser.util.page-ref page-ref
              logseq.graph-parser.util.block-ref block-ref
              logseq.graph-parser.date-time-util date-time-util
-             medley.core medley}}
+             medley.core medley
+             promesa.core p}}
 
   :namespace-name-mismatch {:level :warning}
   :used-underscored-binding {:level :warning}}

docs/dev-practices.md

@@ -139,6 +139,18 @@ To write a test that uses a datascript db:
 
 For examples of these tests, see `frontend.db.query-dsl-test` and `frontend.db.model-test`.
 
+### Async Unit Testing
+
+Async unit testing is well supported in ClojureScript.
+https://clojurescript.org/tools/testing#async-testing is a good guide for how to
+do this. We have a couple of test helpers that make testing async easier:
+
+- `frontend.test.helper/deftest-async` - `deftest` for async tests that ensures
+  uncaught exceptions don't abruptly end the test suite. If you don't use this
+  macro for async tests, you are expected to handle unexpected failures in your test
+- `frontend.test.helper/with-reset` - A version of `with-redefs` that works for
+  async contexts
+
 ## Accessibility
 
 Please refer to our [accessibility guidelines](accessibility.md).

src/main/frontend/handler/plugin_config.cljs

@@ -23,7 +23,7 @@ This component depends on TODO"
 
 (def common-plugin-keys
   "Vec of plugin keys to store in plugins.edn and to compare with installed-plugins state"
-  (->> plugin-config-schema/Plugins-edn last rest (mapv first)))
+  (->> plugin-config-schema/Plugin rest (mapv first)))
 
 (defn add-or-update-plugin
   [{:keys [id] :as plugin}]

src/main/frontend/schema/handler/plugin_config.cljc

@@ -1,4 +1,21 @@
-(ns ^:bb-compatible frontend.schema.handler.plugin-config)
+(ns ^:bb-compatible frontend.schema.handler.plugin-config
+  "Malli schemas for plugin-config")
+
+; The plugin keys should not be changed between releases without a migration plan
+; for existing config files
+(def Plugin
+  [:map
+   [:name
+    [:and {:gen/fmap '(partial str "Name ")}
+     string?]]
+   [:version
+    [:and
+     {:gen/fmap '(fn [_] (apply str (interpose "." (repeatedly 3 (fn [] (rand-int 10))))))}
+     string?]]
+   [:repo
+    [:and {:gen/fmap '(partial str "github-user/")}
+     string?]]
+   [:theme boolean?]])
 
 (def Plugins-edn
   [:map-of
@@ -8,17 +25,4 @@
     {:gen/schema :qualified-keyword
      :gen/fmap '(fn [x] (keyword (str "id-" (name x))))}
     :keyword]
-   ; The plugin keys should not be changed between releases without a migration plan
-   ; for existing config files
-   [:map
-    [:name
-     [:and {:gen/fmap '(partial str "Name ")}
-      string?]]
-    [:version
-     [:and
-      {:gen/fmap '(fn [_] (apply str (interpose "." (repeatedly 3 (fn [] (rand-int 10))))))}
-      string?]]
-    [:repo
-     [:and {:gen/fmap '(partial str "github-user/")}
-      string?]]
-    [:theme boolean?]]])
+   Plugin])

src/test/frontend/fs/test_node.cljs

@@ -1,13 +1,17 @@
 (ns frontend.fs.test-node
   "Test implementation of fs protocol for node.js"
   (:require [frontend.fs.protocol :as protocol]
-            ["fs" :as fs-node]))
+            ["fs/promises" :as fsp]
+            [promesa.core :as p]))
 
 ;; Most protocol fns are not defined. Define them as needed for tests
 (defrecord NodeTestfs
   []
   protocol/Fs
   (read-file [_this _dir path _options]
-             (str (fs-node/readFileSync path)))
+             (p/let [content (fsp/readFile path)]
+                    (str content)))
   (write-file! [_this _repo _dir path content _opts]
-               (fs-node/writeFileSync path content)))
+               (fsp/writeFile path content))
+  (stat [_this _dir path]
+        (fsp/stat path)))

src/test/frontend/fs_test.cljs

@@ -0,0 +1,45 @@
+(ns frontend.fs-test
+  (:require [clojure.test :refer [is use-fixtures]]
+            [frontend.test.fixtures :as fixtures]
+            [frontend.test.helper :as test-helper :include-macros true :refer [deftest-async]]
+            [frontend.fs :as fs]
+            [promesa.core :as p]
+            ["fs" :as fs-node]
+            ["path" :as path]))
+
+(use-fixtures :once fixtures/redef-get-fs)
+
+(deftest-async create-if-not-exists-creates-correctly
+  ;; dir needs to be an absolute path for fn to work correctly
+  (let [dir (path/resolve (test-helper/create-tmp-dir))
+        some-file (path/join dir "something.txt")]
+
+    (->
+     (p/do!
+      (fs/create-if-not-exists nil dir some-file "NEW")
+      (is (fs-node/existsSync some-file)
+          "something.txt created correctly")
+      (is (= "NEW"
+             (str (fs-node/readFileSync some-file)))
+          "something.txt has correct content"))
+
+     (p/finally
+      (fn []
+        (fs-node/unlinkSync some-file)
+        (fs-node/rmdirSync dir))))))
+
+(deftest-async create-if-not-exists-does-not-create-correctly
+  (let [dir (path/resolve (test-helper/create-tmp-dir))
+        some-file (path/join dir "something.txt")]
+    (fs-node/writeFileSync some-file "OLD")
+
+    (->
+     (p/do!
+      (fs/create-if-not-exists nil dir some-file "NEW")
+      (is (= "OLD" (str (fs-node/readFileSync some-file)))
+          "something.txt has not been touched and old content still exists"))
+
+     (p/finally
+      (fn []
+        (fs-node/unlinkSync some-file)
+        (fs-node/rmdirSync dir))))))

src/test/frontend/handler/export_test.cljs

@@ -1,9 +1,8 @@
 (ns frontend.handler.export-test
-  (:require [cljs.test :refer [async deftest use-fixtures are is]]
+  (:require [cljs.test :refer [async use-fixtures are is]]
+            [frontend.test.helper :as test-helper :include-macros true :refer [deftest-async]]
             [clojure.edn :as edn]
             [frontend.handler.export :as export]
-            [frontend.test.helper :as test-helper]
-            [frontend.handler.repo :as repo-handler]
             [frontend.state :as state]
             [promesa.core :as p]))
 
@@ -18,50 +17,48 @@
                   id:: 61506712-3007-407e-b6d3-d008a8dfa88b
                 - ((61506712-3007-407e-b6d3-d008a8dfa88b))
 - 4
-  id:: 61506712-b8a7-491d-ad84-b71651c3fdab"
-    }
+  id:: 61506712-b8a7-491d-ad84-b71651c3fdab"}
    {:file/path "pages/page2.md"
     :file/content
     "- 3
   id:: 97a00e55-48c3-48d8-b9ca-417b16e3a616
         - {{embed [[page1]]}}"}])
 
-(defn- import-test-data!
-  []
-  (repo-handler/parse-files-and-load-to-db! (state/get-current-repo) test-files {:re-render? false}))
-
 (use-fixtures :once
   {:before (fn []
              (async done
-                    (test-helper/clear-current-repo)
-                    (p/let [_ (import-test-data!)]
+                    (test-helper/start-test-db!)
+                    (p/let [_ (test-helper/load-test-files test-files)]
                       (done))))
    :after test-helper/destroy-test-db!})
 
-(deftest export-blocks-as-markdown
-  (are [expect block-uuid-s]
-      (= expect
-         (export/export-blocks-as-markdown (state/get-current-repo) [(uuid block-uuid-s)] "dashes" []))
-    "- 1  \n\t- 2  \n\t\t- 3  \n\t\t- 3  "
-    "61506710-484c-46d5-9983-3d1651ec02c8"
+(deftest-async export-blocks-as-markdown
+  (p/do!
+   (are [expect block-uuid-s]
+        (= expect
+           (export/export-blocks-as-markdown (state/get-current-repo) [(uuid block-uuid-s)] "dashes" []))
+        "- 1  \n\t- 2  \n\t\t- 3  \n\t\t- 3  "
+        "61506710-484c-46d5-9983-3d1651ec02c8"
 
-    "- 3  \n\t- 1  \n\t\t- 2  \n\t\t\t- 3  \n\t\t\t- 3  \n\t- 4  "
-    "97a00e55-48c3-48d8-b9ca-417b16e3a616"))
+        "- 3  \n\t- 1  \n\t\t- 2  \n\t\t\t- 3  \n\t\t\t- 3  \n\t- 4  "
+        "97a00e55-48c3-48d8-b9ca-417b16e3a616")))
 
-(deftest export-files-as-markdown
-  (are [expect files]
-      (= expect
-         (@#'export/export-files-as-markdown (state/get-current-repo) files true))
-    [["pages/page1.md" "- 1  \n\t- 2  \n\t\t- 3  \n\t\t- 3  \n- 4  "]]
-    [{:path "pages/page1.md" :content (:file/content (nth test-files 0)) :names ["page1"] :format :markdown}]
+(deftest-async export-files-as-markdown
+  (p/do!
+   (are [expect files]
+        (= expect
+           (@#'export/export-files-as-markdown (state/get-current-repo) files true))
+        [["pages/page1.md" "- 1  \n\t- 2  \n\t\t- 3  \n\t\t- 3  \n- 4  "]]
+        [{:path "pages/page1.md" :content (:file/content (nth test-files 0)) :names ["page1"] :format :markdown}]
 
-    [["pages/page2.md" "- 3  \n\t- 1  \n\t\t- 2  \n\t\t\t- 3  \n\t\t\t- 3  \n\t- 4  "]]
-    [{:path "pages/page2.md" :content (:file/content (nth test-files 1)) :names ["page2"] :format :markdown}]))
+        [["pages/page2.md" "- 3  \n\t- 1  \n\t\t- 2  \n\t\t\t- 3  \n\t\t\t- 3  \n\t- 4  "]]
+        [{:path "pages/page2.md" :content (:file/content (nth test-files 1)) :names ["page2"] :format :markdown}])))
 
-(deftest export-repo-as-edn-str
-  (let [edn-output (edn/read-string
-                    (@#'export/export-repo-as-edn-str (state/get-current-repo)))]
-    (is (= #{:version :blocks} (set (keys edn-output)))
-        "Correct top-level keys")
-    (is (= ["page1" "page2"] (map :block/page-name (:blocks edn-output)))
-        "Correct pages")))
+(deftest-async export-repo-as-edn-str
+  (p/do!
+   (let [edn-output (edn/read-string
+                     (@#'export/export-repo-as-edn-str (state/get-current-repo)))]
+     (is (= #{:version :blocks} (set (keys edn-output)))
+         "Correct top-level keys")
+     (is (= ["page1" "page2"] (map :block/page-name (:blocks edn-output)))
+         "Correct pages"))))

src/test/frontend/handler/plugin_config_test.cljs

@@ -1,6 +1,6 @@
 (ns frontend.handler.plugin-config-test
   (:require [clojure.test :refer [is use-fixtures testing deftest]]
-            [frontend.test.helper :as helper :include-macros true :refer [deftest-async]]
+            [frontend.test.helper :as test-helper :include-macros true :refer [deftest-async]]
             [frontend.test.fixtures :as fixtures]
             [frontend.handler.plugin-config :as plugin-config]
             [frontend.handler.global-config :as global-config-handler]
@@ -16,7 +16,7 @@
 (use-fixtures :once fixtures/redef-get-fs)
 
 (deftest-async add-or-update-plugin
-  (let [dir (helper/create-tmp-dir)
+  (let [dir (test-helper/create-tmp-dir)
         plugins-file (path/join dir "plugins.edn")
         plugin-to-add {:id :foo :name "Foo" :repo "some-user/foo" :version "v0.9.0"}
         body (pr-str (mg/generate plugin-config-schema/Plugins-edn {:size 10}))]
@@ -31,11 +31,12 @@
 
      (.finally
       (fn []
+        (reset! global-config-handler/root-dir nil)
         (fs-node/unlinkSync plugins-file)
         (fs-node/rmdirSync dir))))))
 
 (deftest-async remove-plugin
-  (let [dir (helper/create-tmp-dir)
+  (let [dir (test-helper/create-tmp-dir)
         plugins-file (path/join dir "plugins.edn")
         ;; use seed to consistently generate 5 plugins
         ;; if we want more randomness we could look into gen/such-that
@@ -53,17 +54,18 @@
 
      (.finally
       (fn []
+        (reset! global-config-handler/root-dir nil)
         (fs-node/unlinkSync plugins-file)
         (fs-node/rmdirSync dir))))))
 
 (deftest-async open-sync-modal-malformed-edn
-  (let [dir (helper/create-tmp-dir)
+  (let [dir (test-helper/create-tmp-dir)
         plugins-file (path/join dir "plugins.edn")
         error-message (atom nil)]
     (fs-node/writeFileSync plugins-file "{:id {}")
     (reset! global-config-handler/root-dir dir)
 
-    (helper/with-reset reset
+    (test-helper/with-reset reset
       [notification/show! (fn [msg _] (reset! error-message msg))]
       (->
        (p/do!
@@ -72,18 +74,19 @@
             "User sees correct notification"))
        (p/finally (fn []
                     (reset)
+                    (reset! global-config-handler/root-dir nil)
                     (fs-node/unlinkSync plugins-file)
                     (fs-node/rmdirSync dir)))))))
 
 (deftest-async open-sync-modal-invalid-edn
-  (let [dir (helper/create-tmp-dir)
+  (let [dir (test-helper/create-tmp-dir)
         plugins-file (path/join dir "plugins.edn")
         error-message (atom nil)]
     ;; Missing a couple plugin keys
     (fs-node/writeFileSync plugins-file (pr-str {:id {:theme true :repo "user/repo"}}))
     (reset! global-config-handler/root-dir dir)
 
-    (helper/with-reset reset
+    (test-helper/with-reset reset
       [notification/show! (fn [msg _] (reset! error-message msg))]
       (->
        (p/do!

src/test/frontend/modules/outliner/core_test.cljs

@@ -10,10 +10,10 @@
             [clojure.walk :as walk]
             [logseq.graph-parser.block :as gp-block]
             [datascript.core :as d]
-            [frontend.test.helper :as helper]
+            [frontend.test.helper :as test-helper]
             [clojure.set :as set]))
 
-(def test-db helper/test-db)
+(def test-db test-helper/test-db)
 
 (use-fixtures :each
   fixtures/load-test-env

src/test/frontend/test/fixtures.cljs

@@ -7,7 +7,7 @@
             [frontend.fs.test-node :as test-node]
             [frontend.fs :as fs]
             [frontend.state :as state]
-            [frontend.test.helper :as helper]
+            [frontend.test.helper :as test-helper]
             [cljs.test :refer [async]]))
 
 (defn load-test-env
@@ -30,7 +30,7 @@
 
 (defn reset-db
   [f]
-  (let [repo helper/test-db]
+  (let [repo test-helper/test-db]
     (reset-datascript repo)
     (let [r (f)]
       (reset-datascript repo) r)))

src/test/frontend/test/helper.clj

@@ -8,9 +8,11 @@
      (frontend.state/set-config! repo# nil)))
 
 ;; Copied from https://github.com/babashka/nbb/blob/e5d84b0fac59774f5d7a4a9e807240cce04bf252/test/nbb/test_macros.clj
-;; This macro ensures an async test's uncaught exception is caught and correctly
-;; errors the test suite. It also handles calling async and done
-(defmacro deftest-async [name opts & body]
+(defmacro deftest-async
+  "A wrapper around deftest that handles async and done in all cases.
+  Importantly, it prevents unexpected failures in an async test from abruptly
+  ending a test suite"
+  [name opts & body]
   (let [[opts body]
         (if (map? opts)
           [opts body]

src/test/frontend/test/helper.cljs

@@ -1,7 +1,6 @@
 (ns frontend.test.helper
   "Common helper fns for tests"
   (:require [frontend.handler.repo :as repo-handler]
-            [frontend.state :as state]
             [frontend.db.conn :as conn]
             ["path" :as path]
             ["fs" :as fs-node]))
@@ -16,14 +15,15 @@
   []
   (conn/destroy-all!))
 
-(defn clear-current-repo []
-  (let [current-repo (state/get-current-repo)]
-    (destroy-test-db!)
-    (conn/start! current-repo)))
-
-(defn load-test-files [files]
-  (repo-handler/parse-files-and-load-to-db! test-db files {:re-render? false :verbose false}))
+(defn load-test-files
+  [files]
+  (repo-handler/parse-files-and-load-to-db!
+   test-db
+   files
+   ;; Set :refresh? to avoid creating default files in after-parse
+   {:re-render? false :verbose false :refresh? true}))
 
 (defn create-tmp-dir
   []
+  (when-not (fs-node/existsSync "tmp") (fs-node/mkdirSync "tmp"))
   (fs-node/mkdtempSync (path/join "tmp" "unit-test-")))