openservicebrokerapi / osb-checker

An automatic checker to verify an Open Service Broker API implementation against the specification
https://github.com/openservicebrokerapi/servicebroker/
Apache License 2.0
48 stars 40 forks source link

Update deprovision test case in test.js #41

Closed leonwanghui closed 5 years ago

leonwanghui commented 6 years ago

Signed-off-by: leonwanghui wanghui71leon@gmail.com

This patch is proposed mainly for resolving issue #33 with some changes below:

cfdreddbot commented 6 years ago

Hey leonwanghui!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

zhongyi-zhang commented 5 years ago

This PR looks good to solve the issue. But in a higher view, I do think the config structure needs a redesign to make more sense, so that broker authors can customize it easier. The running order for these test cases is not obvious, e.g. deprovision is mixed into provisions in current config file but actually it runs after the binding cases below. My proposal is that: each case focus on a service with a plan. Broker authors customize only a valid lifecycle and optional validations. For error-expected cases, OSB checker should be able to generate them e.g. the "conflict" scenario case in current config file -- it's not hard to generate invalid request body from a author-provided valid body.

Just draft a pseudo case for that:

{
  "name": "my abc service test",
  "service_id" :"blabla",
  "plan_id": "blabla",
  "organization_guid": "org-guid-here",
  "space_guid": "space-guid-here",
  "lifecycle": [
    {
      "operation": "provision",
      "async": true
      "parameters": {
        "blabla": "blabla"
      },
    },
    {
      "operation": "update",
      "async": true
      "parameters": {
        "blabla": "blabla"
      },
    },
    {
      "operation": "bind",
      "parameters": {
        "blabla": "blabla"
      },
      "expectedCredentialSchema": { // optional
        "blablakey": "<string>"
      }
    },
    {
      "operation": "unbind",
    },
    {
      "operation": "deprovision",
      "async": true
    }
  ]
}

Of course test.js would need a refactor to adapt these changes.

norshtein commented 5 years ago

I agree with @zhongyi-zhang , the new version is more neat and readable.

zhongyi-zhang commented 5 years ago

Went through other PRs and found that my proposal impacts a lot. I think we can discuss more about it in a new issue. If it is popular, the change will close several PRs and issues.

leonwanghui commented 5 years ago

Agreed, so I will keep this PR opened till the redesign proposal been finished, is that ok?