seancorfield / polylith-external-test-runner

An external (subprocess) test runner for Polylith
Apache License 2.0
13 stars 0 forks source link

Support option to include tests in :src paths as well #6

Closed seancorfield closed 4 months ago

seancorfield commented 9 months ago
tengstrand commented 7 months ago

I added issue 448 issue to the polylith repo.

seancorfield commented 7 months ago

Why? That has nothing to do with this issue.

tengstrand commented 7 months ago

I didn't double check, but I thought we passed in the :test settings to the test runners, and that the :include-src-dirs could be useful.

seancorfield commented 7 months ago

Ah, I see what you're suggesting now... Yes, test-settings is passed in:

(defn create
  [{:keys [workspace project changes #_test-settings]}]
  (let [{:keys [bases components]} workspace
        {:keys [name namespaces paths
                bricks-to-test projects-to-test]} project
        {:keys [project-to-bricks-to-test project-to-projects-to-test]
         :or {project-to-bricks-to-test {name bricks-to-test}
              project-to-projects-to-test {name projects-to-test}}} changes

Is that structure documented somewhere that test runner authors can read?

Since different test runners are going to potentially want all sorts of different configuration options passed through, would Polylith support arbitrary keys inside the :test settings structure? e.g., a test runner like mine could say "put any options for this test runner in :org.corfield.external-test-runner/options {} inside :test in workspace.edn (since you can have different test runners for different projects, right?).

tengstrand commented 7 months ago

Now we have three official test runners, the default, Kaocha, and External. Others may be added in the future. To make it easier for users of the poly tool to switch between test runners, we could use the same name of some of the common settings. Test runner specific settings could maybe live under the :external and :kaocha (if needed) keys, e.g. {:test {:kaocha {:kaocha-specific-key "value"}, :external {:external-test-runner-specific-key "another-value"}}} to eliminate the risk of name clashes.

The documentation mentions that the :test-settings is passed in to the test runner here, but we should add that it also merges the test settings from the project, and maybe that the test runner could use settings under :test for their own test runner.

The :test key is missing in the workspace settings and should be added.

The :test key is documented for projects here, but we need to complement with the :create-test-runner key.

This is just an idea and I'm open to suggestions.

seancorfield commented 7 months ago

I would strongly encourage the use of qualified keys for 3rd party extensions (test runners) but leave it up to those 3rd parties what the names should be -- otherwise, Polylith's documentation will need updating every time a new extension is created out there. I think it is perhaps reasonable for Polylith's docs to include a bulleted list of known extensions (test runners) and link to their docs -- but not actually proscribe any specific keys for their use.

In addition, if Polylith still wants to validate the :test structure and highlight invalid keys (not sure if that is a goal?), then making 3rd parties use qualified keys would mean that Polylith could safely do (into {} (filter (comp namespace key)) settings) to get just the unqualified keys for its own use and validation (while still merging everything and passing everything to the extensions).

seancorfield commented 7 months ago

If :test in workspace.edn contains a :org.corfield/external-test-runner key, that's treated as an options hash map for this test runner. Polylith doesn't seem to object to additional keys like that (I was concerned it might disallow that).

A qualified key like this won't clash with anything else -- other test runners can do the same sort of thing.

For now, I've added support for an :include-src-dir (true/false) option (as part of this ticket). If Polylith does add direct support for this in the built-in test runner at some future point, I'll update this runner to support both options. I'll probably not document this until I work on #5