Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

kensa test provision - Question about "Check all keys in the manifest are present" test #62

Open
mmay opened this issue Dec 4, 2013 · 4 comments

Comments

@mmay
Copy link
Contributor

mmay commented Dec 4, 2013

What is the rational behind the "Check all keys in the manifest are present" test when running kensa test provision? Defined here

e.g.

Check all keys in the manifest are present [FAIL]
    BLAH_URL is missing from the manifest

If I understand this correctly, this checks that any config vars being set by the add-on are returned in the initial response from the add-on provision call.

My problem is that when our add-on is provisioned, we need to set our config var to the app's heroku domain. (e.g. 123xyz.herokuapp.com). Since the add-on provider api does not enable this call until after the add-on is provisioned (makes sense), we really don't want to return any value for our config var as it might break something in our user's app, should they have already set up the config for our add-on. The way we are currently doing this is to set the config var on a heroku/resources update call. But, this causes our kensa provision tests to fail.

So, my question is: why does kensa seem to require that our add-on return values for the config vars we need to set up on the provision call?

Any feedback on this situation?

@bjeanes
Copy link
Contributor

bjeanes commented Jan 14, 2014

@mmay, I think the API may not be as strict about this as Kensa is but the reasoning is that this tests that you meet the interface you are declaring in the manifest. @glenngillen probably has more knowledge about this, but it does seem like setting a subset of config vars could/should be OK. Setting undeclared vars is obviously never OK.

@mmay
Copy link
Contributor Author

mmay commented Jan 14, 2014

That makes sense and seems reasonable.

I do agree: undeclared vars are bad. However, in my case, kensa seems to prefer undeclared vars over no vars, which doesn't seem right to me.

Thanks for the quick merge 👍

@glenngillen
Copy link
Contributor

Good question, but I think that logic predates even me. What it should really be checking is "if you returned config vars, did it only include keys specified in your manifest".

@mmay
Copy link
Contributor Author

mmay commented Jan 18, 2014

Ah, that makes sense. I'm happy to add that logic in when I get time.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants