Upgrade Eastwood

[vemv]

Brief

Long due one!

Some of the changes in Eastwood that are most relevant to f-s:

• New reflection linter that makes our own linter redundant
• The linters that we used to disable have been thoroughly fixed and are not expected to have false positives
• Increased performance and reliability for REPL invocations
• New ways of silencing faults
  • https://github.com/jonase/eastwood#ignored-faults
  • https://github.com/jonase/eastwood#ignoring-linter-sub-kinds

QA plan

For the Eastwood changes, I'd recommend linting large projects and verifying/fixing that the newly enabled linters aren't being problematic. Please report any false positives.

For the runner change, one can (formatting-stack.core/format) and observe that the background runner keeps doing its job.

Author checklist

• [x] I have QAed the functionality
• [x] The PR has a reasonably reviewable size and a meaningful commit history
• [x] I have run the branch formatter and observed no new/significative warnings
• [x] The build passes
• [x] I have self-reviewed the PR prior to assignment
• Additionally, I have code-reviewed iteratively the PR considering the following aspects in isolation:
  • [x] Correctness
  • [x] Robustness (red paths, failure handling etc)
  • [x] Test coverage
  • [x] Spec coverage
  • [x] Documentation
  • [x] Security
  • [x] Performance
  • [x] Breaking API changes
  • The runner change is "breaking" (changed a type from Future to Thread), at the same that ns is quite clearly not an API
  • e.g. we've never received a GH issue about the runner, so presumably nobody is hacking runner
  • [x] Cross-compatibility (Clojure/ClojureScript)

Reviewer checklist

• [ ] I have checked out this branch and reviewed it locally, running it
• [ ] I have QAed the functionality
• [ ] I have reviewed the PR
• Additionally, I have code-reviewed iteratively the PR considering the following aspects in isolation:
  • [ ] Correctness
  • [ ] Robustness (red paths, failure handling etc)
  • [ ] Test coverage
  • [ ] Spec coverage
  • [ ] Documentation
  • [ ] Security
  • [ ] Performance
  • [ ] Breaking API changes
  • [ ] Cross-compatibility (Clojure/ClojureScript)

[vemv]

Amended to use into

[vemv]

Removed the redundant config file. Strangely that revealed an unrelated failing test. I got to fix it by moving it to a different ns, prevening the git-cloning fixture from being run for it.

[thumbnail]

Strangely that revealed an unrelated failing test.

Yeah I looked into this for a bit and am not sure what is actually happening. Moving the test out of this test-ns works but doesn't fix the root cause (maybe the refresh-within-refresh-dirs-only-strategy doesn't work properly in the other git folder?). I'll look into this a bit further.

[vemv]

The fixture was making all-files (sut/all-files :files []) return files such as these:

/home/circleci/repo/git-integration-testing/src/formatting_stack/formatters/cljfmt.clj

whereas they were expected to be like this:

/home/circleci/repo/src/formatting_stack/formatters/cljfmt.clj

...so it seems to me that sut/namespaces-within-refresh-dirs-only is doing its job properly as git-integration-testing/src is not a refresh dir.

[thumbnail]

That makes sense; the test-fixture sets a different CWD to make sure the git-commands run in the right CWD; but that obviously doesn't work for the refresh dirs.

I'm currently checking why the test succeeded before a805eb0

edit: it seems to have found specifically (/Users/.../formatting-stack/git-integration-testing/resources/eastwood/config/formatting_stack.clj). Because that file doesn't have a namespace declaration 😅. That test was truly hanging by a thread.

Anyway; I'm not a big fan of the test being moved out of the 'idiomatic' namespace, we could just set refresh-dirs to a value during the test. This also makes sure we control the 'hidden' variable (refresh-dirs that is):

diff --git a/test/integration/formatting_stack/strategies.clj b/test/integration/formatting_stack/strategies.clj
index 01d0dbc..b7be982 100644
--- a/test/integration/formatting_stack/strategies.clj
+++ b/test/integration/formatting_stack/strategies.clj
@@ -5,6 +5,7 @@
    [clojure.set :as set]
    [clojure.string :as string]
    [clojure.test :refer [deftest is testing use-fixtures]]
+   [clojure.tools.namespace.repl :refer [refresh-dirs]]
    [formatting-stack.strategies :as sut]
    [formatting-stack.test-helpers :as test-helpers :refer [git-integration-dir]]
    [formatting-stack.util :refer [rcomp]]
@@ -231,16 +232,18 @@
         (cleanup-testing-repo!)))))

 (deftest namespaces-within-refresh-dirs-only
-  (speced/let [^{::speced/spec (rcomp count (partial < 100))}
-               all-files (sut/all-files :files [])
-               result (sut/namespaces-within-refresh-dirs-only :files all-files)]
-    (is (seq result)
-        "Returns non-empty results (since f-s itself has namespaces within `src`, `test`, etc)")
-
-    (is (< (count result)
-           (count all-files))
-        "Doesn't include files outside the refresh dirs")
-
-    (is (set/subset? (set result)
-                     (set all-files))
-        "Is a subtractive (and not additive) mechanism")))
+  (with-redefs [refresh-dirs [(str (io/file git-integration-dir "src"))
+                              (str (io/file git-integration-dir "test"))]]
+   (speced/let [^{::speced/spec (rcomp count (partial < 100))}
+                all-files (sut/all-files :files [])
+                result    (sut/namespaces-within-refresh-dirs-only :files all-files)]
+     (is (seq result)
+         "Returns non-empty results  (since f-s itself has namespaces within `src`, `test`, etc)")
+
+     (is (< (count result)
+            (count all-files))
+         "Doesn't include files outside the refresh dirs")
+
+     (is (set/subset? (set result)
+                      (set all-files))
+         "Is a subtractive (and not additive) mechanism"))))

[vemv]

Rebased and added a minor commit

[vemv]

Added a commit following PR feedback, and removed the old one

[vemv]

Rebased again, now we have 3 commits. The first one includes thesilence feedback

[thumbnail]

Thanks a lot for the patience and the work!