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.

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

Integrant elides Midje metaconstants from config #31

Closed
WhittlesJr opened this issue Nov 30, 2017 · 10 comments
Closed

Integrant elides Midje metaconstants from config #31

WhittlesJr opened this issue Nov 30, 2017 · 10 comments

Comments

@WhittlesJr
Copy link

WhittlesJr commented Nov 30, 2017

(First of all, I'm loving Integrant! It's changed the way I look at application architecture fundamentally. Thank you for all of your hard work!)

My use case is that I test the initialization and halting of my Integrant keys using config containing Midje metaconstants.

The below:

(require '[integrant.core :as ig]
         '[midje.sweet :refer :all])

(defmethod ig/init-key :some/component
  [_ config]
  (println config))

(fact
  (ig/init {:some/component {:config-key ..value..}})

  => {:some/component {:config-key ..value..}})

Produces:

{:config-key {}}

FAIL at (*cider-repl some-namespace*:479)
    Expected:
{:some/component {:config-key ..value..}}
      Actual:
{:some/component nil}
       Diffs: in [:some/component] expected {:config-key ..value..}, was nil
@weavejester
Copy link
Owner

Sorry, I don't understand what you're asking.

You've created a failing test, and Midje tells you that it's failed. Is there some additional information in the failure message that you expect to see?

@WhittlesJr
Copy link
Author

My bad, I should have been more specific. My expectation was that the test should pass as written, but instead the metaconstant is turned into an empty map. I'm trying to look around to see where this would happen.

I noticed that the config's ::origin metadata does retain the metaconstant as given:

{:integrant.core/build {:some/component {:config-key {}}}
 :integrant.core/origin {:some/component {:config-key ..value..}}}

@WhittlesJr
Copy link
Author

er, sorry, I see your confusion now better. The fact that my init-key method doesn't actually return anything useful is seemingly incidental to the problem of metaconstants disappearing. I'll try to come up with a better example

@weavejester
Copy link
Owner

Your init-key method for :some/component returns nil, so running init will produce:

{:some/component nil}

This seems in line with the failure message you're getting.

@WhittlesJr
Copy link
Author

Here, this is more what I'm getting at:

(defmethod ig/init-key :some/component
  [_ config]
  (:config-key config))

(fact
  (ig/init {:some/component {:config-key ..value..}})

  => {:some/component ..value..})

Produces:

Error Metaconstants (..value..) can't be compared for equality with {}.

Whereas the following passes:

(fact
  (ig/init {:some/component {:config-key "value"}})

  => {:some/component "value"})

@weavejester
Copy link
Owner

Isn't this a problem with Midje? Integrant doesn't produce anything unusual; the output of init is an immutable map with some metadata attached to provide halt ordering.

@WhittlesJr
Copy link
Author

I'm really not sure. In practice, my init-key and halt-key! multimethods all only have a single call to an external function that handles all the config destructuring and logic. So since Midje works just fine with those regular functions, my test coverage probably probably doesn't need to include my Integrant multimethods.

I just thought it was strange that the metaconstants would behave this way when passed through ig/init.
Stranger yet, even if you put the defmethod inside of a fact macro, this still happens.

Seems like a limitation that could be avoided somehow, but I'm not sure if I should be asking you or @marick

@weavejester
Copy link
Owner

I don't think this is an issue with Integrant. init is a function that takes in a map and returns a map with metadata - there's nothing particularly unusual about it, and a test library should be able to handle that. I'd suggest opening this issue on the Midje project.

@WhittlesJr
Copy link
Author

Alright, I'll do that. Thank you!

@WhittlesJr
Copy link
Author

Well, it seems the issue is not easily fixed (if it's even possible to fix it). I still don't understand why this is happening, but I'll just give up and work around it.

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

No branches or pull requests

2 participants