Skip to content

Restore clj-github.test-helpers/build-spec behavior#40

Merged
marcobiscaro2112 merged 6 commits into
mainfrom
mb/fix-regression
Jun 19, 2026
Merged

Restore clj-github.test-helpers/build-spec behavior#40
marcobiscaro2112 merged 6 commits into
mainfrom
mb/fix-regression

Conversation

@marcobiscaro2112

Copy link
Copy Markdown
Contributor

build-spec was changed to return quoted forms instead of plain data, which breaks some use cases (such as clj-github.state-flow-helper).

This reverts build-spec to its old behavior, fixing breakage in clj-github.state-flow-helper as well as some direct calls from clients (now documented as not recommended).

It also adds a basic test case for clj-github.state-flow-helper (that would detect the issue).

`build-spec` was changed to return quoted forms instead of plain data, which breaks some use cases (such as `clj-github.state-flow-helper`).

This reverts `build-spec` to its old behavior, fixing breakage in `clj-github.state-flow-helper` as well as some direct calls from clients (now documented as not recommended).
Copilot AI review requested due to automatic review settings June 18, 2026 16:07
(:import (java.util.regex Pattern)))


(defn- spec-type [spec]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplifies cond + defmethods into a simple cond (as this is not open for external extension)

Comment on lines -37 to -45
(defn -add-default-content-type
[response]
(assoc-in response [:headers :content-type] "application/json"))

(defn add-default-content-type [response]
`(let [responder# (fake/responder ~response)]
(fn [origin-fn# opts# callback#]
((or callback# identity)
(responder# origin-fn# opts# -add-default-content-type)))))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inlined

((or callback identity)
(responder orig-fn opts force-json)))))

(defn ^:internal build-spec

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returns plain data again, instead of quoted forms

`(fake/with-fake-http ~(build-spec spec)
~@body))
`(fake/with-fake-http
(build-spec ~(mapv #(if (-> % meta :path) `{:path ~%} %) spec))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preserve :path as intended (feature from 0.8.0 release)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR restores clj-github.test-helpers/build-spec to return plain runtime values (instead of quoted forms), fixing breakage in clj-github.state-flow-helper’s mock-github-flow and adding a regression test to catch the issue in the future.

Changes:

  • Reworked build-spec to produce concrete request/response specs (including responder functions) suitable for http-kit-fake at runtime.
  • Updated with-fake-github to preserve the ^:path “computed path” feature while still feeding build-spec plain data.
  • Added a basic mock-github-flow test and bumped the library version.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/clj_github/state_flow_helper_test.clj Adds regression coverage ensuring mock-github-flow honors explicit response specs and returns parsed JSON bodies.
src/clj_github/test_helpers.clj Restores runtime (non-quoted) build-spec behavior and adapts with-fake-github to keep computed-path support.
project.clj Bumps version to 0.9.1-beta1.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/clj_github/test_helpers.clj Outdated
It was missing the leading slash in the mocked URL (no it was not actually mocked by default).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal workflows won't run in public repos like this, they always error out, so there is no point in keeping them

@marcobiscaro2112 marcobiscaro2112 merged commit 1e05cd3 into main Jun 19, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants