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

Keyword arguments not used when resolving its documentation and tags … #3566

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

VernonCrabtree
Copy link

#3238
The code has been modified to resolve the arguments before executing the keyword. This required splitting the StatusReporter context manager so that any resolution errors could be put into the reporting context.
All tests have passed except Telnet tests.
New tests have been added for tags with parameters.

@codecov-io
Copy link

codecov-io commented May 1, 2020

Codecov Report

Merging #3566 into master will decrease coverage by 0.06%.
The diff coverage is 69.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3566      +/-   ##
==========================================
- Coverage   74.21%   74.15%   -0.05%     
==========================================
  Files         199      200       +1     
  Lines       16466    16578     +112     
  Branches     2653     2672      +19     
==========================================
+ Hits        12218    12292      +74     
- Misses       3817     3844      +27     
- Partials      431      442      +11     
Impacted Files Coverage Δ
src/robot/output/xmllogger.py 95.54% <37.50%> (-3.53%) ⬇️
src/robot/output/logcontroller.py 64.64% <64.64%> (ø)
src/robot/running/userkeywordrunner.py 59.05% <79.49%> (+1.68%) ⬆️
src/robot/output/logger.py 92.53% <100.00%> (+0.14%) ⬆️
src/robot/output/output.py 87.50% <100.00%> (+0.84%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e05b85...8646e8d. Read the comment docs.

Vernon Crabtree added 2 commits May 1, 2020 21:57
@pekkaklarck
Copy link
Member

I don't have time to really look at the PR now, but on a quick look it seems rather complicated. It actually seems so complicated that the functionality provided by it may not be worth the increased maintenance cost. This is especially true because in the next RF major version we plan to refactor execution side code and we most likely need to touch this code as well.

@VernonCrabtree
Copy link
Author

Oh well - it was fun trying. I had realized it was complex, which is why I deliberately tried to contain the change to one module and reduced the complexity from where I had started.
Maybe the acceptance tests can be used in the next release?
Also, when looking at refactoring, this code could be worth a quick look to help determine the issues that need to be addressed. It should be easier once python 2 no longer needs to be supported.

@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2020

Codecov Report

Merging #3566 into master will decrease coverage by 0.07%.
The diff coverage is 69.13%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3566      +/-   ##
==========================================
- Coverage   74.21%   74.15%   -0.06%     
==========================================
  Files         199      200       +1     
  Lines       16466    16590     +124     
  Branches     2653     2675      +22     
==========================================
+ Hits        12218    12300      +82     
- Misses       3817     3847      +30     
- Partials      431      443      +12     
Impacted Files Coverage Δ
src/robot/output/xmllogger.py 92.38% <40.00%> (-6.70%) ⬇️
src/robot/output/logcontroller.py 66.67% <66.67%> (ø)
src/robot/running/userkeywordrunner.py 59.05% <79.49%> (+1.68%) ⬆️
src/robot/output/logger.py 92.53% <100.00%> (+0.14%) ⬆️
src/robot/output/output.py 87.50% <100.00%> (+0.84%) ⬆️
src/robot/running/builder/builders.py 90.00% <0.00%> (ø)
src/robot/running/handlers.py 74.90% <0.00%> (+0.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e05b85...0fab471. Read the comment docs.

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

4 participants