Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

should contain keys: summary could be improved [0.7.2] #155

Closed
hlship opened this issue Apr 9, 2019 · 3 comments
Closed

should contain keys: summary could be improved [0.7.2] #155

hlship opened this issue Apr 9, 2019 · 3 comments
Labels
bug

Comments

@hlship
Copy link

hlship commented Apr 9, 2019

The should contain keys summary of key specs for composites (via s/keys) could be better.

-- Spec failed --------------------

  {:tire-line-numbers [1 2 3 4],
   :store-id "5504",
   :last-modified-at "2019-04-04T17:02:26.000 0000",
   :start-time "10:00",
   :order-number "26698749828094",
   :end-time "10:30",
   :store-time-zone "PST",
   :reservation-id "3c59240d-4fc3-438a-8707-05091a02a93f",
   :booked-at "2019-04-20T10:00:00.000 0000",
   :installation-line-number 5,
   :delivery-at "2019-04-20T14:00:00.000 0000"}

should contain keys: :customer, :store-address

|            key |                                                                                            spec |
|---------------- -------------------------------------------------------------------------------------------------|
|      :customer |                                                        (keys :req-un [string? string? string?]) |
| :store-address | (keys :req-un [string? (and string? (fn [%] (re-matches #"\d{5}(-d{4})?" %))) string? string?]) |

-- Relevant specs -------

:eReceipts.query-service.order-service-proxy-client/add-tire-installation-item-details:
  (clojure.spec.alpha/keys
   :req-un
   [:eReceipts.query-service.order-service-proxy-client/order-number
    :eReceipts.query-service.order-service-proxy-client/last-modified-at
    :eReceipts.query-service.order-service-proxy-client/tire-line-numbers
    :eReceipts.query-service.order-service-proxy-client/installation-line-number
    :eReceipts.query-service.order-service-proxy-client/store-time-zone
    :eReceipts.query-service.order-service-proxy-client/store-id
    :eReceipts.query-service.order-service-proxy-client/store-address
    :eReceipts.query-service.order-service-proxy-client/booked-at
    :eReceipts.query-service.order-service-proxy-client/start-time
    :eReceipts.query-service.order-service-proxy-client/end-time
    :eReceipts.query-service.order-service-proxy-client/delivery-at
    :eReceipts.query-service.order-service-proxy-client/reservation-id
    :eReceipts.query-service.order-service-proxy-client/customer])

-------------------------
Detected 1 error

Given:

(s/def ::address (s/keys :req-un [::street-address
                                  ::postal-code
                                  ::state
                                  ::city]))

(s/def ::street-address string?)
(s/def ::postal-code (s/and string?
                            #(re-matches #"\d{5}(-d{4})?" %)))
(s/def ::state string?)
(s/def ::city string?)

(s/def ::store-address ::address)

I'd expect the table to reference those qualified spec names, rather than what they expand into.
@bhb
Copy link
Owner

bhb commented Apr 12, 2019

@hlship Thanks for reporting this!

Can you provide an example of what you'd expect the table to contain?

@bhb
Copy link
Owner

bhb commented Apr 12, 2019

Just to clarify, what's the spec for ::customer?

I think I see the problem - are you saying that the table should show the spec, but instead of:

| :store-address | (keys :req-un [string? (and string? (fn [%] (re-matches #"\d{5}(-d{4})?" %))) string? string?]) |

it should include something like this?

| :store-address | (s/keys :req-un [:eReceipts.query-service.order-service-proxy-client/street-address :eReceipts.query-service.order-service-proxy-client/postal-code :eReceipts.query-service.order-service-proxy-client/state :eReceipts.query-service.order-service-proxy-client/city]) |

@bhb bhb added the bug label Jun 25, 2019
@bhb bhb mentioned this issue Aug 1, 2019
Merged
@bhb
Copy link
Owner

bhb commented Nov 25, 2019

@hlship Thanks for reporting this! The fix is included in version 0.8.0, which I've just released.

@bhb bhb closed this as completed Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bhb @hlship