Skip to content
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

#497 - fix timeout step hanging if no visual automation #498

Merged
merged 22 commits into from Jul 12, 2019

Conversation

kensoh
Copy link
Member

@kensoh kensoh commented Jul 12, 2019

In commit 5cba169 from issue #443 and PR #453 the goal is to also update SikuliX integration internal timeout variable. However there is a bug introduced that makes timeout step hangs if visual automation is not used. Raising an issue to fix this bug with a PR.

kensoh added 22 commits June 10, 2019 11:12
SikuliX wait_timeout also needs to be changed as part of sikuli_step() when timeout step is used.

Otherwise, when present() or visible() is invoked, the SikuliX timeout setting gets reverted back to its internal variable.

Also, 'casper' object should be used in displaying the error message instead of 'this'. Otherwise, for the execution context, this would be undefined and error message does not show up.
In newer DevTools Protocol, Page.captureScreenshot now supports clip parameter. This allows directly specifying the region to capture for a web element, instead of the previous implementation where a full screen is captured and then clipped as a PhantomJS webpage.

This improves performance as only the data of the selector snapshot is needed to be transferred between TagUI and Chrome instead of the whole webpage. Also, as PhantomJS is not involved, the quality and color would be more accurate without additional web profile of PhantomJS.

Lastly, this improvement also fixes an issue with snap step in live mode for Chrome, where the snapshot captures full screen instead of only the selector.
this change adds support for escape characters in live mode, for eg when using keyboard step ' will give error and requires creative syntax to type a '

idea for solution is from solving a similar problem during recent development of personal side project - tagui for python
Making a commit to improve on the existing script to kill TagUI processes.

For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser.

This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state.

These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
Follow-up update to previous commit below -

Making a commit to improve on the existing script to kill TagUI processes.

For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser.

This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state.

These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
Follow-up update to previous 2 commits below -

Making a commit to improve on the existing script to kill TagUI processes.

For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser.

This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state.

These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
Follow-up update to previous 3 commits below by improving to remove Linux warning message -

Making a commit to improve on the existing script to kill TagUI processes.

For eg if Ctrl+C is used to kill TagUI prematurely, TagUI main process would lose the control to do clean-up of integrations (SikuliX, R, Python, Chrome) and Chrome browser.

This shell script (for all the 3 OSes) will kill those processes to free up memory and allows TagUI to start running from a clean state.

These actions are not done by default when TagUI is launched, as it is expected that TagUI exits cleanly. Otherwise cleaning up all processes by default will hide potential issues should they arise and they will end up not getting reported.
More details at issue aisingapore#490. Only 7% of files are not formatted for easy reading. It makes no sense to refactor these 7% (3 files tagui_header.js, tagui_parse.php, translate.php) at a risk of hurting existing compatibility and scripts. Solution is to provide beautified snapshots of these 3 files and add link to these snapshots at top of the files, so that any developer who wishes to view or edit the source code can see the beautified version.

As TagUI is an engine that tries to translate human languages into working JavaScript code that works with its various components, it has mash-up code using string manipulations and replacements in order to get some things done. Simply using a beautifier may break some parts of the engine that affects some existing users' scripts. Thus it is better to provide beautified versions for the 3% of files that is hard to read, then try to refactor at risk of breaking existing users' scripts.
In commit aisingapore@5cba169 from issue aisingapore#443 and PR aisingapore#453 the goal is to also update SikuliX integration internal timeout variable. However there is a bug introduced that makes timeout step hangs if visual automation is not used. Raising an issue to fix this bug with a PR.
@kensoh kensoh merged commit b19c510 into aisingapore:master Jul 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant