Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
test: fixes to call-forwarding scripts #208
Conversation
alfonsosanchezbeato
reviewed
Sep 28, 2015
| - for p in properties: | ||
| + for p in ["VoiceBusy", "VoiceNoReplyTimeout", "VoiceNotReachable", | ||
| + "VoiceUnconditional", "VoiceNoReply", | ||
| + "ForwardingFlagOnSim"]: |
alfonsosanchezbeato
Sep 28, 2015
Contributor
Why did you do this change? The set you use between the brackets is exactly the same contained in "properties".
tonyespy
Sep 28, 2015
Contributor
I did this so that I could get rid of the first test case in:
https://wiki.ubuntu.com/Process/Merges/TestPlans/ofono/CallForwarding
This change now causes the script to verify that the interface and all the properties are properly exported.
alfonsosanchezbeato
Sep 28, 2015
Contributor
Got it, but taking into account that that is Ubuntu Touch's test plan, I am not sure if upstream would accept this change if that is the only reason. If we want to fully automatize CallForwarding testing I think we should create a different script similar to the ones you created for testing the SIM interface, because I do not think that test-call-forwarding fully fits our testing needs.
alfonsosanchezbeato
reviewed
Sep 28, 2015
| + | ||
| + try: | ||
| + main(args) | ||
| + except KeyboardInterrupt as e: |
alfonsosanchezbeato
Sep 28, 2015
Contributor
Handling the interrupt is nice, but I think that adding some timeout (~30 secs) to exit the script would be good to have too.
alfonsosanchezbeato
Sep 28, 2015
Contributor
I think making sure it exits would remove the feeling that something went wrong when executing the script. An alternative could be to print something like "Waiting for property changes, press Ctrl+C when finished" if you prefer that.
|
Looks good, although I have a couple of comments. Also, it would be good to handle Ctrl+C in set-call-forwarding in the same way you do for test-call-forwarding, and add a time out to exit for both scripts. Finally, I think it's worth upstreaming this. |
tonyespy commentedSep 22, 2015
Added multi-modem support to set-call-forwarding, changed test-call-forwarding to verify the expected CF properties and implement a KeyboardInterrupt handler so a backtrace isn't output when Ctrl-C is used to terminate the main loop.
Also added cf.SetProperty calls for "VoiceBusy" and "VoiceNotReachable" to fully cover all of the properties.