Support custom statuses #3869

mihaiparvu · 2021-03-02T12:42:45Z

Added the possibility to set custom status from command line option --addstatus. The new status is set based on tags and displayed both in console output and log.html. The color can be customized but only in the log file, in console it maintains the old colors.

…work into issue_3625 # Conflicts: # src/robot/htmldata/rebot/testdata.js # src/robot/reporting/jsmodelbuilders.py # src/robot/result/model.py # src/robot/result/suiteteardownfailed.py # src/robot/result/xmlelementhandlers.py # src/robot/running/statusreporter.py

…work into issue_3625

codecov-io · 2021-03-02T15:03:06Z

Codecov Report

Merging #3869 (f17e11a) into master (0fabf9f) will decrease coverage by 0.12%.
The diff coverage is 48.94%.

@@            Coverage Diff             @@
##           master    #3869      +/-   ##
==========================================
- Coverage   75.27%   75.16%   -0.11%     
==========================================
  Files         222      222              
  Lines       18716    18753      +37     
  Branches     3036     3045       +9     
==========================================
+ Hits        14087    14094       +7     
- Misses       4096     4120      +24     
- Partials      533      539       +6

Impacted Files	Coverage Δ
src/robot/run.py	`80.40% <ø> (ø)`
src/robot/running/suiterunner.py	`74.36% <ø> (ø)`
src/robot/running/status.py	`69.46% <33.34%> (-2.35%)`	⬇️
src/robot/conf/settings.py	`70.77% <42.86%> (-1.51%)`	⬇️
src/robot/running/statusreporter.py	`82.09% <60.00%> (+0.28%)`	⬆️
src/robot/output/console/highlighting.py	`75.94% <100.00%> (+0.75%)`	⬆️
src/robot/reporting/jsmodelbuilders.py	`98.24% <100.00%> (-0.01%)`	⬇️
src/robot/utils/robotinspect.py	`47.62% <0.00%> (-19.04%)`	⬇️
src/robot/version.py	`55.56% <0.00%> (-11.11%)`	⬇️
src/robot/libdocpkg/datatypes.py	`94.57% <0.00%> (-1.08%)`	⬇️
... and 1 more

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 0fabf9f...f17e11a. Read the comment docs.

pekkaklarck

This is a good start but the core logic how custom statuses are set to objects need to be changed to something less fragile. Let's discuss about better alternative separately.

There were also some other issues explained in individual comments. In addition to them, I noticed these problems when testing the code locally:

Statistics aren't collected for the custom statuses. There probably should be new status columns in both log and report statics, but needed to test how that works if there are multiple custom statuses. Should also show custom statuses in status graphs.

Right now also collecting "normal" statistics is broken. I tested creating a new status based on PASS, and those tests were considered failed in all statistics.
Dark status colors make reading the black status text hard in the log file. We preferably should automatically decide should the status text be black or white depending on the specified status color. Alternatively we could require giving that as well but it would make the usage a bit more complicated.

pekkaklarck · 2021-06-23T08:17:33Z

atest/resources/TestCheckerLibrary.py

@@ -136,7 +136,7 @@ def _check_test_status(self, test, status=None, message=None):
            test.exp_status = status
        if message is not None:
            test.exp_message = message
-        if test.exp_status != test.status:
+        if test.exp_status != test.status and test.status in ['PASS', 'FAIL', 'SKIP']:


This change looks risky. Doesn't it mean that custom statuses aren't validated at all?

pekkaklarck · 2021-06-23T08:18:41Z

doc/userguide/src/ExecutingTestCases/log_failed.html

@@ -1322,7 +1322,6 @@
    var idCounter = 0;
    var _statistics = null;
    var LEVELS = ['TRACE', 'DEBUG', 'INFO', 'WARN', 'ERROR', 'FAIL', 'SKIP'];
-    var STATUSES = ['FAIL', 'PASS', 'NOT_RUN', 'SKIP'];


No need to change examples under doc at all. These changes just add distraction to the PR.

pekkaklarck · 2021-06-23T08:23:02Z

src/robot/conf/settings.py

@@ -273,6 +282,13 @@ def _process_tag_stat_link(self, value):
        raise DataError("Invalid format for option '--tagstatlink'. "
                        "Expected 'tag:link:title' but got '%s'." % value)

+    def _process_custom_statuses(self, value):
+        tokens = value.split(':')
+        if len(tokens) == 4 and tokens[1] in ['PASS', 'FAIL', 'SKIP']:


Should support also lower case values.

pekkaklarck · 2021-06-23T08:25:07Z

src/robot/htmldata/rebot/log.html

+  for (let status of Object.keys(statuses)) {
+    css.append(".label.".concat(status.toLowerCase()).concat("{ background-color:").concat(statuses[status]).concat("}"));
+  }
+}


Isn't it possible to just modify CSS directly and not inject <style> like this? If styles are injected, we could probably do that when writing the log file itself.

pekkaklarck · 2021-06-23T08:28:01Z

src/robot/htmldata/rebot/testdata.js

@@ -38,7 +37,7 @@ window.testdata = function () {
    }

    function parseStatus(stats) {
-        return STATUSES[stats[0]];
+        return stats[0];


Statuses should be written to log.html as integers like earlier. That takes less space and lists containing only integers are faster to process by browsers than lists containing integers and strings. The hard coded STATUSES list mapping integers to actual statuses should just be replaced with a list containing also the custom statuses. It probably could be written to the config section along with custom title, background color, etc.

pekkaklarck · 2021-06-23T08:29:46Z

src/robot/output/console/highlighting.py

+            # reset custom statuses
+            StatusMixin.PASS = 'PASS'
+            StatusMixin.FAIL = 'FAIL'
+            StatusMixin.SKIP = 'SKIP'


This looks really strange. Do I get the logic right that code elsewhere sets custom values for StatusMixin.XXX constants and they are reset here? If yes, that's not a very good approach. It only works if code is executed in certain order and doesn't work at all if you programmatically check results.

pekkaklarck · 2021-06-23T08:33:59Z

src/robot/reporting/jsmodelbuilders.py

@@ -60,7 +59,7 @@ def __init__(self, context):
    def _get_status(self, item):
        # Branch status with IF/ELSE, "normal" status with others.
        status = getattr(item, 'branch_status', item.status)
-        model = (STATUSES[status],
+        model = (status,


As I already commented the related JS code on log.html side, statuses should still be integers in log.html. Should somehow be able to map also custom statuses to integers here. Probably the mapping could be passed to JsModelBuilder but also the processes result object could have that info.

pekkaklarck · 2021-06-23T08:37:02Z

src/robot/run.py

+                          Examples:
+                              --addstatus KNOWN_ISSUE:FAIL:bug-id-*:purple
+                              --addstatus NON-CRITICAL:SKIP:non-critical:pink
+                          New in RF 4.x


for|foritem looks like a copy-paste error.

Also rebot.py would need this option.

pekkaklarck · 2021-06-23T08:39:25Z

src/robot/running/status.py

+                        StatusMixin.FAIL = custom_status[0]
+                    else:
+                        StatusMixin.SKIP = custom_status[0]
+                    break


As I commented the code on the highlighter, the logic to here modify constants and then elsewhere resetting them isn't a valid approach. It's too fragile because it requires code to be run in certain exact order and doesn't work at all if analyzing/modifying results programmatically. I'm not exactly sure what's the best way to handle this core logic, but I write some ideas as general comments.

pekkaklarck · 2021-06-23T09:42:44Z

The hard part with this issue is designing how to store information about custom statuses internally. For example, should the status attribute tests have get these custom statuses or should it always be one of the base statuses PASS/FAIL/SKIP? The former initially feels better, but it would require changes in all code looking for statuses. In the latter case test could/should have a separate attribute that returns possible custom status based on configs and test base status and tags. We could consider renaming the current status to e.g. base_status and using status with the new attribute, though.

We also need to design how to pass the custom status information to tests. Most likely it could be given to the top level result suite where tests could query it. We used similar approach with criticality information earlier.

We can discuss all this further here or on Slack. It seems pretty clear that there's so much work still to be done that it's nevertheless best to move this issue from RF 4.1 to RF 5.0 scope.

mparvu and others added 8 commits February 23, 2021 14:31

Add custom status support

23af2e9

Added missing file

3240fd8

Merge branch 'master' into issue_3625

6aca3e2

Merge branch 'master' of https://github.com/robotframework/robotframe…

b999cdc

…work into issue_3625

Update unit tests

e4ae451

Merge remote-tracking branch 'origin/issue_3625' into issue_3625

dfdbc22

Fix for help documentation

f17e11a

Fix for help documentation (trailing space)

709766b

pekkaklarck mentioned this pull request Jun 7, 2021

Support custom statuses #3625

Open

pekkaklarck requested changes Jun 23, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support custom statuses #3869

Support custom statuses #3869

mihaiparvu commented Mar 2, 2021

codecov-io commented Mar 2, 2021 •

edited

pekkaklarck left a comment

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck Jun 23, 2021

pekkaklarck commented Jun 23, 2021

Support custom statuses #3869

Are you sure you want to change the base?

Support custom statuses #3869

Conversation

mihaiparvu commented Mar 2, 2021

codecov-io commented Mar 2, 2021 • edited

Codecov Report

pekkaklarck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pekkaklarck commented Jun 23, 2021

codecov-io commented Mar 2, 2021 •

edited