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

Remove applied test case in update operation #76

Closed leonwanghui closed 5 years ago

leonwanghui commented 5 years ago

Generally this patch will remove Update - applied test case in testUpdate.js with reasons below:

So it's better to keep the broker return 202 for all three scenarios before, and therefore the applied test should be removed.

cfdreddbot commented 5 years ago

:white_check_mark: Hey leonwanghui! The commit authors and yourself have already signed the CLA.

zhongyi-zhang commented 5 years ago

Sorry, I still didn't get your point... Isn't the case removed in the situation that the last update operation is already done after polling https://github.com/openservicebrokerapi/osb-checker/blob/master/common/testUpdate.js#L95-L102?

leonwanghui commented 5 years ago

Hi @mattmcneeney @duglin , we are gonna need your help, thanks!

norshtein commented 5 years ago

The question is: unlike provisioning response can have 200, 201, 202 status code, the updating response can only have 200 and 202 status code. Consider following scenarios:

zhongyi-zhang commented 5 years ago

For async update, it should definitely return 200. For sync update, I think it should return 200, too. In sync update, there is no difference between "a brand new update request" and "a update request which it is already applied". Code 200 just indicates that the operation is done.

mattmcneeney commented 5 years ago

Hey @leonwanghui Do we have a test case for a synchronous update returning a 200 (with accepts_incomplete=false)? Since this is really what is being tested for with expecting a 200 straight away

leonwanghui commented 5 years ago

Yes, please see here

mattmcneeney commented 5 years ago

Great, I think this is fine then!