-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add squishserver module #18
Conversation
Added squishserver module to work with squishserver, that contains a dedicated class SquishServer. Added 3 methods to register AUTs: - addAUT - addAppPath - addAttachableAut Also added dedicated exception type "SquishserverError"
For the completeness of the solution, it might be worth to add "remove" functions for the "add" functions: |
With the recent changes, one can see the need to create a helper function, like: |
Thanks for the recent changes, looks like a big improvement :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pawtom could you please check my comments? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general it would be good to see squishserver details in logs. Lets imagine a situation where you have 2 squishservers. If you register different AUTs in each of them, the logs do not say which AUT was added to which squishserver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please read the comment I made last time
@Adakar @EduardKaverinskyi So instead of doing something like that in the test script:
What do you think? |
I think that this idea is usable. I implemented it in the latest commit. Please check if it is as is suggested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EduardKaverinskyi I'll not accept this PR without implementing more verbose log messages. It was mentioned twice already in this comment
Changes: - Add docstrings to the class and methods - Extend the SquishserverError message to include additional data about failure - Format the code - Reorder the imports
In this commit I have added information about squishserver to the SquishServer class methods. For all methods this information in placed at the very beginning of the log statement.
e567093
to
bd6ef03
Compare
* Add squishserver module Allows attaching to and starting AUT Allows configuring squishserver: - addAUT - removeAUT - addAppPath - removeAppPath - addAttachableAut - removeAttachableAut --------- Co-authored-by: TomaszPe <tomasz.pawlowski@gmail.com>
* Add squishserver module Allows attaching to and starting AUT Allows configuring squishserver: - addAUT - removeAUT - addAppPath - removeAppPath - addAttachableAut - removeAttachableAut --------- Co-authored-by: TomaszPe <tomasz.pawlowski@gmail.com>
Added squishserver module to work with squishserver, which contains a dedicated class SquishServer. Added 3 methods to register AUTs:
Also added a dedicated exception type "SquishserverError"