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

Redesign the config file #50

Closed zhongyi-zhang closed 5 years ago

zhongyi-zhang commented 5 years ago

Solve https://github.com/openservicebrokerapi/osb-checker/issues/48. Also do a refactor on the test framework.

This PR tries to do a minimal change on the test framework to adjust the config redesign. All the test cases are kept. Something that not looks good to me are just commented as TODO. Currently only the mock server can be ensured to pass all the test cases. Will create subsequent issues and PRs to continue improving the checker.

The change in test.js is huge. Git diff works not very well on it. Actually most of the original logic are kept. The refactor only optimizes how to trigger them according to the new config file. Appreciate your reviews!

cfdreddbot commented 5 years ago

Hey zhongyi-zhang!

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.

leonwanghui commented 5 years ago

I think the framework is generally ok, and I noticed there are several TODO items which are related to some other PRs, so I will rebase my PRs after this PR merged.

zhongyi-zhang commented 5 years ago

@leonwanghui thanks for the review. Looking forward to the PRs after rebase. I'll help review. @duglin @fmui @kibbles-n-bytes hi, it seems that the Pull Approve requires at least 2 of your approvals in the list. I'd like to respect the PR merging process. Could you help? Thanks!

duglin commented 5 years ago

LGTM

Approved with PullApprove

zhongyi-zhang commented 5 years ago

Thanks! Now one more approval is required... @mattmcneeney as you know about the redesign, could you approve it? BTW, can @norshtein, @leonwanghui, and me be added to the .pullapprove.yml? It would help the project move faster on the non-critical PRs (e.g. fixing typo) and engineering-only PRs (e.g. fixing logic which is not reflecting the purpose of a case) -- what we are pretty sure that they can be merged.

mattmcneeney commented 5 years ago

@zhongyi-zhang I have added @norshtein and @leonwanghui as approvers. Thanks and have fun!

mattmcneeney commented 5 years ago

@zhongyi-zhang I have added @norshtein and @leonwanghui as approvers. Thanks and have fun!

mattmcneeney commented 5 years ago

@zhongyi-zhang I have added @norshtein and @leonwanghui as approvers. Thanks and have fun!

mattmcneeney commented 5 years ago

@zhongyi-zhang I have added @norshtein and @leonwanghui as approvers. Thanks and have fun!

mattmcneeney commented 5 years ago

@zhongyi-zhang I have added @norshtein and @leonwanghui as approvers. Thanks and have fun!

mattmcneeney commented 5 years ago

@zhongyi-zhang I have added @norshtein and @leonwanghui as approvers. Thanks and have fun!

norshtein commented 5 years ago

LGTM

Approved with PullApprove

cfdreddbot commented 5 years ago

Hey zhongyi-zhang!

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

@mattmcneeney thanks!! Seems the change doesn't have an effect on the existing PR... Just merge it this time.