Closed HackXIt closed 3 weeks ago
Please update Changelog file to upcoming unreleased new keywords.
Is it possible to extend some Screenshots tests under ATests to see if log are resolved?
(Optional would be nice)
Probably a switch would be nice to decide default behavior from screenshots to create files or base64 like
Set Screenshot Log Mode (Base64 | File)
I'll get to address the changes right away, please keep in mind that I noticed that my commits made were using the wrong user configuration by mistake. I have rebased the commits since then to overwrite the author with my correct information.
It should be visible in a moment.
I'm done from my point of view, please have another look @Nepitwin
@HackXIt
Code looks good so far. If tests are all green i will approve your PR. Thank you for the functional enhancement.
@HackXIt
Code looks good so far. If tests are all green i will approve your PR. Thank you for the functional enhancement.
Appveyor seems to take forever since it's going through all the commits one-by-one. Is that necessary?
I'll fix any issues that will come up on tests of the last build.
EDIT: Nevermind, that was my fault, I should not have pushed the commits individually. Bad habit.
@HackXIt Code looks good so far. If tests are all green i will approve your PR. Thank you for the functional enhancement.
Appveyor seems to take forever since it's going through all the commits one-by-one. Is that necessary?
I'll fix any issues that will come up on tests of the last build.
EDIT: Nevermind, that was my fault, I should not have pushed the commits individually. Bad habit.
Each commit will force Appveyor to trigger test execution yes.
Test would be faster on payment model from appveyor to execute all tests parallel but it's only the free variant which force each build one by one.
I haven't gotten around to fixing the tests yet, I'll try to do it tomorrow, since vacation starts on Friday.
EDIT: Did not get to it before vacation, will have to wait until 2nd week of August.
@Nepitwin I've run the tests locally, and they should be working with the latest commit, please check on it once appveyor is done, to finalize this PR.
I had a little bit of a hiccup with properly passing arguments and also returning values through the function chain.
AppVeyor was unable to build non-mergeable pull request
Not really sure what happened there, appveyor did not run. I am unable to force or retry the build. It did not execute.
EDIT: Got it, there was a merge conflict with Changelog.md
@HackXIt
I could help to implement the testcases if it's fine.
Then we can merge and deploy a new version from it.
It triggers me hard that they are not working.
I should be better at this ;D
@HackXIt
Feel free this is the challenge of testing to execute them correctly on a testing system 📦
@Nepitwin I do need some help on this last test case. I do not know what runs differently on appveyor.
When I run the test locally, it creates the screenshot file and finds it correctly, but on appveyor it doesn't. I changed the format to use ${TEST_NAME} instead of hardcoding, but that didn't change results.
By the way, if you also have some suggestions on further necessary test cases, I can add those as well, but I believe with the feature being so small, the ones created should suffice. Here's a list of the new ones:
Attached is my local log file of the successful execution of that test case. I did not run it via the keen.bat
, since it executes quite a lot of test cases and I do not wish to run the full suite to verify a single one. It would be nice to have a mechanism in keen.bat
to at least run suites seperately, but that's a change for another day.
result_take_screenshot_of_window.zip
C:; cd 'C:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui\atests'; & 'C:\Users\RINI\AppData\Local\Temp\rf-ls-run\run_env_00_38e9sp6_.bat' '-u' 'c:\Users\RINI\.vscode\extensions\robocorp.robotframework-lsp-1.12.0\src\robotframework_debug_adapter\run_robot__main__.py' '--port' '7111' '--no-debug' '--listener=robotframework_debug_adapter.events_listener.EventsListenerV2' '--listener=robotframework_debug_adapter.events_listener.EventsListenerV3' '--outputdir' 'C:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui/result/tmp' '--pythonpath' 'c:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui/.venv/lib/python3.8/site-packages' '--pythonpath' 'c:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui/atests' '--variable' 'UIA:UIA3' '--prerunmodifier=robotframework_debug_adapter.prerun_modifiers.FilteringTestsSuiteVisitor' 'c:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui\atests\Screenshot.robot' s\x5cRINI\x5c#Entwicklung_TechNICK\x5c0_Git_Stash\x5crobotframework-flaui\x5catests\x5cScreenshot.robot'
==============================================================================
Screenshot :: Test suite for screenshot keywords. XPath not found error han...
==============================================================================
Take Screenshot Of Window | PASS |
------------------------------------------------------------------------------
Screenshot :: Test suite for screenshot keywords. XPath not found ... | PASS |
1 test, 1 passed, 0 failed
==============================================================================
Output: C:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui\result\tmp\output.xml
Log: C:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui\result\tmp\log.html
Report: C:\Users\RINI\#Entwicklung_TechNICK\0_Git_Stash\robotframework-flaui\result\tmp\report.html
It would be better to always have it in the teardown, as currently it will not be executed on failure for the test that has to also stop the application. I'll add a keyword I think to combine the two.
@Nepitwin I do need some help on this last test case. I do not know what runs differently on appveyor.
When I run the test locally, it creates the screenshot file and finds it correctly, but on appveyor it doesn't. I changed the format to use ${TEST_NAME} instead of hardcoding, but that didn't change results.
By the way, if you also have some suggestions on further necessary test cases, I can add those as well, but I believe with the feature being so small, the ones created should suffice. Here's a list of the new ones:
- Take Screenshot As Base64
- Take Screenshot Of Window (this is the failing one)
- Take Screenshot Of Window As Base64
Attached is my local log file of the successful execution of that test case. I did not run it via the
keen.bat
, since it executes quite a lot of test cases and I do not wish to run the full suite to verify a single one. It would be nice to have a mechanism inkeen.bat
to at least run suites seperately, but that's a change for another day. result_take_screenshot_of_window.zipExecution log
@HackXIt
Run a case in a robotfile, you can execute command like
.\keen.bat install
cd .\atests\
python -m robot --name "UIA3" --variable UIA:UIA3 --outputdir ../result/uia3 --test "Take Screenshot Of Window" .\Screenshot.robot
--test is the casename you edit in robotfile
There seems to be an unrelated problem on AppVeyor. The last commit had all tests successful, but the script had a failure somewhere. See console log for details on AppVeyor.
...
5 File(s) copied
Command exited with code 28
$wc = New-Object 'System.Net.WebClient'
$wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\result\xunit.xml))
There seems to be an unrelated problem on AppVeyor. The last commit had all tests successful, but the script had a failure somewhere. See console log for details on AppVeyor.
... 5 File(s) copied Command exited with code 28 $wc = New-Object 'System.Net.WebClient' $wc.UploadFile("https://ci.appveyor.com/api/testresults/junit/$($env:APPVEYOR_JOB_ID)", (Resolve-Path .\result\xunit.xml))
@HackXIt
I debuged that, job "call:pylint" in keen.bat maybe retrurn the %ERRORLEVEL%=28, so the %result%=28 and it donnot change in left jobs, so keen.bat ended with EXIT /B %result%=28. EXIT code not be 0, it is marked as a Failure.
I add the attachment debug-flaui.zip with debugcode and EXIT code with 0 tip: use keen.bat in attachment and command ".\keen.bat test" will run only ./Screenshot.robot, no other robotfile
EXIT code with 28 in log, job "call:pylint" and job "tidy" print like
EXIT code with 0, should print like
@JimRevolutionist
Alright, applied the changes to keen.bat
. (only the changes relating to errorlevel)
I also adjusted all screenshot test cases with a proper teardown keyword for resetting to default state, which should also run on error to not influence other test cases.
I also ran the screenshot suite beforehand, should all be green.
Fingers crossed, we should be good to go for merge after this.
Do you want me to create issues for the keen.bat so we can reference it in the PR?
I should also update Changelog to reflect the minor adjustments made.
@JimRevolutionist
Alright, applied the changes to
keen.bat
. (only the changes relating to errorlevel)I also adjusted all screenshot test cases with a proper teardown keyword for resetting to default state, which should also run on error to not influence other test cases.
I also ran the screenshot suite beforehand, should all be green.
Fingers crossed, we should be good to go for merge after this.
Do you want me to create issues for the keen.bat so we can reference it in the PR?
I should also update Changelog to reflect the minor adjustments made.
@HackXIt
optmize keen.bat should be an Independent issue,pr separate from screenshot issue,pr. We should not put keenbat commits into screenshot pr.
Oh. I'll revert the change then.
@HackXIt
Oh yes i forgot to update all related verifiction from pylint to see in error logs from appveyor. Sorry for this mistake. If you got already an solution you can create a new pr from it.
Because all issues can be seen afterwards in artifact files to guarantee python code style guides and robotframework has to be fixxed.
@JimRevolutionist
Sometimes api call failed from appveyor --> Command exited with code 28
It's not nice if a run was failed but for this case i would rerun the testcase from my perspective.
Sorry for late response.
@JimRevolutionist
By actuall solution error code is handled but appveyor does not upload zip file :(
ERRORLEVEL after call xcopy uia3 is: 0 ResultCode after call xcopy uia3: 1 ERRORLEVEL after call parsly is: 0 ResultCode after call parsly is: 1 Command exited with code 1 $wc = New-Object 'System.Net.WebClient'
In zip file from artifact the reasons from pylint is described what to fix.
But it can be seen in logs
Module src.FlaUILibrary.flaui.module.screenshot src\FlaUILibrary\flaui\module\screenshot.py:115:8: R1705: Unnecessary "elif" after "return", remove the leading "el" from "elif" (no-else-return) src\FlaUILibrary\flaui\module\screenshot.py:111:4: R1710: Either all return statements in a function should return an expression, or none of them should. (inconsistent-return-statements) Module src.FlaUILibrary.keywords.screenshot src\FlaUILibrary\keywords\screenshot.py:20:0: C0303: Trailing whitespace (trailing-whitespace) src\FlaUILibrary\keywords\screenshot.py:47:0: C0301: Line too long (150/120) (line-too-long) src\FlaUILibrary\keywords\screenshot.py:4:0: W0611: Unused robotlog imported from FlaUILibrary.robotframework (unused-import) ***** Module src.FlaUILibrary.robotframework.robotlog src\FlaUILibrary\robotframework\robotlog.py:48:8: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
Verification locally can be done with
keen.bat pylint
But you should first install new package with
keen.bat install keen.bat pylint
Alright, after all that… I think we finally have a valid one.
I fixed all the pylint errors, so appveyor should also give green light if my luck hasn't forsaken me.
Unfortunately, due to my bad behavior of pushing too much, it will take a bit until it gets to the last commit.
Yay.
Thank you @Nepitwin
I might do another PR soon regarding #199
This PR adds another logging function, to directly embed screenshots as base64 encoded strings in the output log.
It also expands the screenshot functionality to allow for screenshot creation based on a provided element XPATH. It will create a screenshot based on that element, so a screenshot of a window will contain the whole window, a screenshot of a button, will only contain that button.
Fixes #192 and fixes #193