Improve acceptance tests

[Harm10]

Coping with several problems (like older RF versions / possibility that connection was not established / extra enter to bypass password about to expire warning).

As I used RIDE 2.0 to edit the tests I also got some lay-out changes enforced by RIDE.

[codecov-commenter]

Codecov Report

Patch coverage has no change and project coverage change: +0.71 🎉

Comparison is base (bacab18) 94.29% compared to head (9584bec) 95.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   94.29%   95.00%   +0.71%     
==========================================
  Files           4        4              
  Lines         613      621       +8     
  Branches       98       96       -2     
==========================================
+ Hits          578      590      +12     
+ Misses         29       27       -2     
+ Partials        6        4       -2     

Flag	Coverage Δ
acceptance	81.80% <ø> (+2.19%)	⬆️
unit	88.08% <ø> (+1.13%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[robinmatz]

Hey @Harm10 , thank you for the contribution!

Here's my thoughts on this:

• the main issue why the jobs in the pipeline fails is because pub400.com blocks request from IPs if there are too many requests. This is what is causing the "Host disconnected" error you can see in the pipeline. Currently, the only workaround I see for this would be to change the configuration in .gitbub/workflows/run-tests.yml to only run on pull requests. However, I also like the benefit of seeing the pipeline run before making a PR, so I would like to keep that.

• I don't think it makes much of a difference if the test teardown fails too if no connection was opened, the result is that the test case failed. But maybe we can make sure that no other clean up steps are run after the Close Connection keywords.

• as for the tests Send Enter and Send PF, I had taken a look at them previously. Here is my suggestion: how about we change those so that there is no need for a user on pub400.com. For example if you send enter and there is no credentials, there is an error message that we could validate. The same goes for Send PF. Usually there is an error message that say "function key not allowed" which also indicates that a PF was sent.

• we currently do not define a minimum robot framework version we support, and for the reason mentioned above I would not like to include more jobs in the pipeline, so I think it would be okay if we used syntax features from the newest robot framework version

• lastly, this project uses robotidy, which is run during the inv lint command, to keep code formatting consistent across multiple developers. Changing this because of individual IDE choice doesn't seem justified to me

[Harm10]

@robinmatz My thoughts on your thoughts :-):

I did not have problems with pub400 blocking my requests. Perhaps doing a proper login beforehand makes sure of that?

Teardown failing only gives noise on the interpretation of the test in my view.
Perhaps you could solve it by not letting Close Connection throw an error if there is no connection at all?

I think it is a good idea not having a userid involved (that would also solve the "problem" of the messages being in another language if your user is set to another language!
Are you sure the PF send also works when you not have logged in yet?

I do not agree with your approach to the version of RF. As far as I am aware the current version still supports the lower ones so using a command like RETURN which is new seems to be out of question to me.

I forgot to do inv lint after I changed the code. So you are right that you shouldn't use RIDE and leave it like that.
I merely used it because I could not find a way to only execute a specific test in atest. At least I could not find the proper synteax for it.

Having said this: do I still need to do something to update this PR?

[robinmatz]

Hi @Harm10 , I opened a PR on your repository with my suggested changes.

[Harm10]

I will review your changes this evening.

[Harm10]

@robinmatz I have tested your merge in my PR and all seems to work well! Big improvement that you do not need an userid of pub400.

[robinmatz]

Thanks alot for the improvements in the tests as well as in the documentation!