Request

I'd like to request a new linter rule. Currently the rule TooManyTestSteps applies to regular Test Cases as well as test-template test cases; I'd like to restrict TooManyTestSteps to apply only to regular Test Cases and a new, separate rule TooManyTemplateDataRows to apply to only to test-templates.

Reasoning

A new rule would be more consistent with the Robot Framework documentation. When a regular test case is too long, it has "too many steps", but test-templates are different. According to the User Guide section on test-templates, a test-template body consists of data rows, not "steps", so the TooManyTestSteps rule technically doesn't apply to a templated test case. A separate TooManyTemplateDataRows rule for templated test cases would be more consistent with existing RobotFramework documentation.

Additionally, it may be desirable to restrict templated test cases to a different length than regular test cases. When regular test cases have too many steps, they're dense and unreadable. But the bare, tabular format of the test-template syntax can contain many more data rows and still look clean. A separate rule would give developers the option to keep Test Cases short and sweet while allowing templates to breathe a little bit.

Defaults

Since TooManyTestSteps defaults to 10, a TooManyTemplateDataRows rule could sensibly default to 15.

When the '-r' recursive argument is given on the command line, rflint seems to analyse files more than once. At least some files appear multiple times in the output.
How often it appears seems to depend on the depth of the folder the file is in.
e.g. below, we can see that files that are immediately under the supplied folder get processed once, files that are a level lower get processed twice, a level below that 4 times, a level below that 8 times, ...

Below is output from a bash script I ran. Because no two lines have the same number of slashes (the slashes from the file paths), we can see that all files at the same depth show up the same number of times. The number at the start of each line of output is the number of times that files at that level got repeated.

$ rflint -r ./project1/ | grep '^\+' | sort | uniq -c | sort -n |  sed 's/\/[^/]*/\//g ' | uniq 
      1 + .//
      2 + .///
      4 + .////
      8 + ./////

Based on those numbers, and looking at the code. It looks like it is because you are using os.walk, which gets all descendant files/folders; but then in the handling for each folder, (_process_folder), you are calling os.walk again; which is not necessary, because the top call to os.walk has gone down to all descendant folders, not just child folders.

Currently this won't pay attention to the [Template] setting on the test itself, only to the 'Test Template' setting on the parent file. (Also doesn't account for colon in the parent suite's setting name)

should probably be updated to something like:

@property
def is_templated(self):
    """Return True if the test is part of a suite that uses a Test Template"""
    for setting in self.settings:
        if setting[1].lower() == '[template]':
            return True
    for table in self.parent.tables:
        if isinstance(table, SettingTable):
            for row in table.rows:
                if row[0].lower().rstrip(':') == "test template":
                    return True
    return False

I don't know whether this affects any of the built in rules; but while writing some rules of my own, I used ResourceFile.settings, which uses SimpleTableMixin.statements.

One of my .robot files had a *** Settings *** table, with only two blank lines after it and no settings.

This seemed to get through to the last while loop in SimpleTableMixin.statements, where it popped off those two lines, but tried to keep going.

I suggest that

        # trim trailing blank statements                                                                                                                                                                       
        while (len(statements[-1]) == 0 or
	       ((len(statements[-1]) == 1) and len(statements[-1][0]) == 0)):
            statements.pop()

needs to look more like

        # trim trailing blank statements                                                                                                                                                                       
        while (len(statements) > 0) and (
               (len(statements[-1]) == 0 or
	       ((len(statements[-1]) == 1) and len(statements[-1][0]) == 0))
        ):
            statements.pop()

I propose a new rule be added to detect instances of trailing whitespace on a line.

(rflint) ✘-1 /opt/robotframework [master|…1⚑ 1]
12:27 $ python -m rflint ./atest/testdata/standard_libraries/deprecated_os/files/latin-1.txt
+ ./atest/testdata/standard_libraries/deprecated_os/files/latin-1.txt
Traceback (most recent call last):
  File "/usr/local/Cellar/python/2.7.8_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/local/Cellar/python/2.7.8_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/opt/robotframework-lint/rflint/__main__.py", line 5, in <module>
    app.run(sys.argv[1:])
  File "/opt/robotframework-lint/rflint/rflint.py", line 78, in run
    suite = RobotFileFactory(filename)
  File "/opt/robotframework-lint/rflint/parser/parser.py", line 38, in RobotFileFactory
    rf = RobotFile(path, parent)
  File "/opt/robotframework-lint/rflint/parser/parser.py", line 68, in __init__
    self._load(path)
  File "/opt/robotframework-lint/rflint/parser/parser.py", line 86, in _load
    for linenumber, raw_text in enumerate(Utf8Reader(f).readlines()):
  File "/Users/gkisel/.virtualenvs/rflint/lib/python2.7/site-packages/robotframework-2.8.6-py2.7.egg/robot/utils/utf8reader.py", line 44, in readlines
    yield self._decode(line, remove_bom=index == 0)
  File "/Users/gkisel/.virtualenvs/rflint/lib/python2.7/site-packages/robotframework-2.8.6-py2.7.egg/robot/utils/utf8reader.py", line 49, in _decode
    return content.decode('UTF-8')
  File "/Users/gkisel/.virtualenvs/rflint/lib/python2.7/encodings/utf_8.py", line 16, in decode
    return codecs.utf_8_decode(input, errors, True)
UnicodeDecodeError: 'utf8' codec can't decode byte 0xe4 in position 3: invalid continuation byte

This looks pretty similar to #13

To quote from the robotframework user guide:

All setting names can optionally include a colon at the end, for example Documentation:. This can make reading the settings easier especially when using the plain text format. This is a new feature in Robot Framework 2.5.5.

But, RequireSuiteDocumentation will not recognise the documentation setting with a colon after the name.

Could perhaps change
if row[0].lower() == "documentation":
to
if row[0].lower().rstrip(':') == "documentation":

It would be nice to be able to say -i *Documentation* as opposed to -i RequireKeywordDocumentation -i RequireSuiteDocumentation -i RequireTestDocumentation

It might be useful to have a warning for files that are more than N lines long. A reasonable default might be 1,000 lines or so. When files get too long they should probably be broken up into multiple shorter files.

Currently all scanned files are listed. If you scan a lot of files you end up having to search for errors in between all the files that have no errors. Example:

+ ./atest/testdata/misc/multiple_suites/01__suite_first.robot
+ ./atest/testdata/misc/multiple_suites/02__sub.suite.1/first__suite4.robot
+ ./atest/testdata/misc/multiple_suites/02__sub.suite.1/second__.Sui.te.2..robot
W: 0, 0: '.' in suite name 'second__.Sui.te.2.' (PeriodInSuiteName)
+ ./atest/testdata/misc/multiple_suites/03__suite3.robot
+ ./atest/testdata/misc/multiple_suites/04__suite4.robot
+ ./atest/testdata/misc/multiple_suites/05__suite5.robot
+ ./atest/testdata/misc/multiple_suites/10__suite10.robot
+ ./atest/testdata/misc/multiple_suites/SUite7.robot
+ ./atest/testdata/misc/multiple_suites/suiTe_8.robot
+ ./atest/testdata/misc/multiple_suites/suite 6.robot
+ ./atest/testdata/misc/multiple_suites/suite_9_name.robot
+ ./atest/testdata/misc/normal.robot
+ ./atest/testdata/misc/pass_and_fail.robot
+ ./atest/testdata/misc/setups_and_teardowns.robot
+ ./atest/testdata/misc/suites/__init__.robot
+ ./atest/testdata/misc/suites/fourth.robot
+ ./atest/testdata/misc/suites/subsuites/sub1.robot
+ ./atest/testdata/misc/suites/subsuites/sub2.robot
+ ./atest/testdata/misc/suites/subsuites2/sub.suite.4.robot
W: 0, 0: '.' in suite name 'sub.suite.4' (PeriodInSuiteName)
+ ./atest/testdata/misc/suites/subsuites2/subsuite3.robot
+ ./atest/testdata/misc/suites/tsuite1.robot
+ ./atest/testdata/misc/suites/tsuite2.robot
+ ./atest/testdata/misc/suites/tsuite3.robot
+ ./atest/testdata/misc/timeouts.robot
+ ./atest/testdata/misc/unicode.robot
+ ./atest/testdata/misc/warnings_and_errors.robot
E: 10, 0: Duplicate testcase name 'Warning in test case' (DuplicateTestNames)
+ ./atest/testdata/output/flatten_keywords.robot
+ ./atest/testdata/output/js_model.robot
+ ./atest/testdata/output/listeners/test_template.robot
W: 4, 0: Unknown table name 'TestCases' (InvalidTable)
+ ./atest/testdata/output/names_needing_escaping.tsv
+ ./atest/testdata/parsing/bom.robot
+ ./atest/testdata/parsing/bom.tsv
+ ./atest/testdata/parsing/data_formats/mixed_data/TSV.tsv
+ ./atest/testdata/parsing/data_formats/mixed_data/TXT.txt

If you provide suite paths to rflint with wildcards, they do not work on Windows.

In POSIX, I guess shell does the wildcard expansion for you, but this is not the case in Windows where neither cmd or powershell will do that for you.

Therefore, on Windows I cannot write:

> rflint -A my_args.robot tests/prefix_*.robot

When I use non-ASCII symbols in test-case, test-suite names, tags, etc. which are included in rflint output, text of UnicodeError appears instead of rflint message
(environment: python 2.7)

Test-case example:

*** Test Cases ***

 Пример тест-кейса.
    [Tags]   example
    Log   Test-case with non-ASCII symbols in its name.

rflint output:

$ rflint example.robot 
+ example.robot
'ascii' codec can't encode characters in position 22-27: ordinal not in range(128)

Expected output:

$ rflint example.robot 
+ example.robot
W: 14, 0: '.' in testcase name 'Пример тест-кейса.' (PeriodInTestName)

I have a resource rule that needs to process a json configuration file. I pass this file in via the --configure option. I verified that the class is being picked up by the linter by executing the rflint --list.

However, when I execute the rflint command, it says the rule is unknown. If I just change the abstract class that my concrete class is subclassed from to GeneralRule, then the rule is actually applied by rflint.

Here's an example class.

class LibraryImport(ResourceRule):
    severity = ERROR

    def configure(self, jsonfile):
        self.jsonfn = jsonfile
        self.allowed_libs = json.load(open(jsonfile,'r'))


    def apply(self,resource):
        settings_table = [table for table in resource.tables if table.name == 'Settings']
        if not settings_table:
            return 
        else:
            settings_table = settings_table[0]
        for row in settings_table.rows:
            if re.search('library',row[0],re.IGNORECASE) and row[1] not in self.allowed_libs:
                msg = "A library '{0}' is being imported in a resource file but this library is not allowed to be used. Allowed libraries are: {1}".format(row[1],self.allowed_libs)
                self.report(resource,msg,row.linenumber)

According to https://code.google.com/p/robotframework/wiki/HowToWriteGoodTestCases#Workflow_tests test cases should not contain complex logic like for loops or if statements. Detecting for loops seems pretty easy, but detecting if/else and variable assignments seems like it might be a little trickier unless you hardcode a list of disallowed keywords, such as Run Keyword If.

Instead of having to run python -m rflint myTestSuite.robot it would be more user friendly to just run rflint myTestSuite.robot. Usually in your setup.py you'd add something like scripts=[ 'bin/rflint']. I'm trying to do this in my fork, but I'm getting this error:

$ rflint
Traceback (most recent call last):
  File "/Users/gkisel/.virtualenvs/rflint/bin/rflint", line 10, in <module>
    execfile(__file__)
  File "/opt/robotframework-lint/bin/rflint", line 3, in <module>
    import rflint.rflint as rflint
  File "/Users/gkisel/.virtualenvs/rflint/bin/rflint.py", line 10, in <module>
    execfile(__file__)
  File "/opt/robotframework-lint/rflint/rflint.py", line 26, in <module>
    from .common import SuiteRule, TestRule, KeywordRule, GeneralRule, Rule, ERROR, WARNING
ValueError: Attempted relative import in non-package

I think this should be pretty easily doable, though. I'm probably just missing something obvious.

I was thinking it would be nice to have a rule that warns about code duplication, or about the potential to pull reused code out into a separate keyword.

I've made an initial attempt at it, but I was wondering if anyone else thinks this seems like something worth doing, or if it would just raise too many false positives to be useful.

guykisel@7412f5c

Imagine we have a lot of TagRule objects with a lot of common code. It could be interesting to create a first rule with the common code and then inherit from it to make the other rules. Actually, this is not possible when using a single inheritance because of _getrules:

    def _get_rules(self, cls):
        """Returns a list of rules of a given class
        
        Rules are treated as singletons - we only instantiate each
        rule once. 
        """

        result = []
        for rule_class in cls.__subclasses__():
            rule_name = rule_class.__name__.lower()
            if rule_name not in self._rules:
                rule = rule_class(self)
                self._rules[rule_name] = rule
            result.append(self._rules[rule_name])
return result

for rule_class in cls.__subclasses__() only iterate over subclasses. So we have to do something like:

class TagTest(TestRule):
    def __init__(self, controller, severity=None):
        super().__init__(controller, severity)

    def configure(self, test_tags):
        self.test_tags = test_tags.split(',')

    def apply(self, testcase):
        continue

class TagScope(TagTest, TestRule): # Ugly hack for cls.subclasses
    def apply(self, testcase):
        continue

Improvement idea: use isinstance() instead of iterating over direct subclasses.

class Foo:
    foo = 1

class Bar(Foo):
    bar = 1

class Foobar(Bar):
    foobar = 1

isinstance(Foobar(), Foo) # True

When I iterate through a set of test case steps, each step's list starts with an empty cell. Note this empty item precedes the first cell in the actual robotframework table. Expectation would be that first cell contain the first 'real' item in the row.

Example output from rflint when iterating through steps.

(5-5)[u'', u'Log', u'this is a test case.']
(6-6)[u'']
(8-8)[u'', u'@{VALUES}=', u'Create List', u'1', u'2', u'4']
(9-9)[u'', u':FOR', u'${var}', u'IN', u'@{VALUES}']
(10-10)[u'', u'\\', u'Log', u'${var}']

Here is the test case table it was iterating through.

*** Test Cases ***

test case one
    [tags]   logging  bar
    Log   this is a test case.

test case two
    @{VALUES}=  Create List   1   2   4
    :FOR    ${var}  IN  @{VALUES}
    \   Log  ${var}

--configure TooFewKeywordSteps:1

This keyword returns TooFewKeywordSteps warning

My second keyword
    My first keyword    # call My first keyword

When I print out keyword.steps, it returns empy list (= [])

There should be a way to get a description of a rule, such as "rflint --describe LineTooLong", which will print the docstring for that rule.

If you give rflint a bad filename, it should throw a warning to stderr.

Comments that start at the left margin are thrown away. We need to preserve them, so rules can check for them (ie: some people might want to discourage such comments, but we can't write that rule if we throw those comments away)

I would like to customize the LineTooLong rule by overwriting it in a site_rules directory that is local to the project.

I have a custom LineTooLong rule but it doesn't load unless the class name is different from the built-in one. I've tried first ignoring the built-in LineTooLong rule with the -i flag and then loading the custom rule with the -R flag. However, rflint then just ignores both rules.

Overall, I'm trying to implement a way to ignore linting a rule on a line that ends with:

#rflint_ignore rule_name

Is there any way I can add this functionality?

Hi, thanks for your working firstly.
I'm trying to walk every keyword in the suite folder to find out similar keywords in different resource file.
But I was not able to add rules base on suite folder class.
I don't understand how to pass the folder class into apply method in the rule class.
Is it possible to add rules for folder? Any reply would be greatly appreciated!
@guykisel @boakley

In addition to #43 , currently RfLint does not allow to use Robot argument file (it does not ignore non rflint compatible params,it does not read suits and paths).

When #43 would be implemented, this would issue would wrap around nicely Robot input parameters.

It would be nice to be able to load a rule file by filename, mainly for testing while developing the rule.

I suggest --rulefile, -R, which takes a filename as an argument.

--list gives a list of rule names, but it would be nice if you could optionally get a verbose listing that has both the name and the docstring for the rule.

It would be nice if the table headings were an attribute of the table object, so that it
is easy to write a rule that enforces a specific style (eg: *** Settings *** vs.
* Settings *)

I need to test all the files in the projects and get a report for all .robot in one file, is this possible ?
Another thing, can we add other rules and integrate it with SonarQube ?

Hi
currently RfLint accepts either path to filename or folder with recursive search.
It would be very useful to allow user to use Robot format for TestSuites as well so the same arguments for Robot could be used in RfLint:
What I mean is to:

• allow use of --suite/-s switch together with project path: http://robotframework.org/robotframework/latest/RobotFrameworkUserGuide.html#by-test-suite-and-test-case-names together with project path
• allow to use -t to specify which test from suite to check

There are rules to require each atom contain rules (RequireTestDocumentation and RequireKeywordDocumentation),
and one to require test suite file documentation (RequireSuiteDocumentation),
but not one to require that a keyword resource file contain documentation. Could a rule be added to require this, e.g. RequireResourceDocumentation or RequireLibraryDocumentation or some such?

Tests or keywords containing zero steps should probably be marked as warnings or errors.

+ ./atest/testdata/parsing/nbsp.robot
Traceback (most recent call last):
  File "/usr/local/Cellar/python/2.7.8_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/local/Cellar/python/2.7.8_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/opt/robotframework-lint/rflint/__main__.py", line 5, in <module>
    app.run(sys.argv[1:])
  File "/opt/robotframework-lint/rflint/rflint.py", line 81, in run
    rule.apply(suite)
  File "/opt/robotframework-lint/rflint/rules/suiteRules.py", line 29, in apply
    self.report(suite, "Unknown table name '%s'" % table.name, table.linenumber)
  File "/opt/robotframework-lint/rflint/common.py", line 25, in report
    char=char_offset)
  File "/opt/robotframework-lint/rflint/rflint.py", line 111, in report
    rulename = rulename, char=char)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xa0' in position 20: ordinal not in range(128)

https://github.com/robotframework/robotframework/blob/master/atest/testdata/parsing/nbsp.robot is the file that's failing to parse, I think.

REQUIREMENT: I need a keyword in my test whose implementation will be done in future, so I added keyword with 'No Operation'.

Example:

*** Keywords ***
Keyword Name
| |No Operation

QUESTION: Is it possible to ignore keyword with 'No Operation' in (TooFewKeywordSteps)?

It would be nice to be able to pass simple arguments to rules. For example, consider a rule to count the number of steps in a test. Right now the only option is to hard-code some value (or write a bunch of code for the rule to look up the value somewhere).

It would be nice to be able to specify it right on the command line. For example, if I wanted the TooManySteps rule to trigger on 20 lines I might specify it like this:

$ rflint -warning TooManySteps:20

This brings up the question, what if someone does this:

$ rflint --warning TooManySteps:20 --error TooManySteps:30

That seems to imply you want a warning if there are more than 20, but an error if there are more than 30. Is that what we want it to mean, or should the --error option override the --warning option?

Could you provide a single option for turning all warnings into errors and exiting the program with non-zero status?

TagWithSpaces always returns the line number of the test case, rather than the line number where the tag actually appears.

When I write a custom rule, the wiki [1] says this: Custom rules should be put in the site-rules folder.

Is my assumption correct, that it should be in $PYTHONPATH/rflint/site-rules folder? Can it be changed to somewhere else, if I want to put those files more easily in version control system?

[1] https://github.com/boakley/robotframework-lint/wiki/How-to-write-custom-rules

I like to write rules to copy the problematic cell into the message for self.report().

If the problematic code includes unicode, then this results in an error

'ascii' codec can't encode character u'\xf3' in position 104: ordinal not in range(128)

I think this stems from RfLint.format() where it calls self.args.format.format().
But, self.args.format is an ascii string, so when formatting, it can't insert the unicode text into it.

I tried to define a GeneralRule class. According to the documentation here, this means I will be passed a RobotFile class when the apply method defined in the class is executed.
https://github.com/boakley/robotframework-lint/wiki/The-GeneralRule-class

According to the documentation here, the RobotFile class is super class of SuiteFile and ResourceFile
https://github.com/boakley/robotframework-lint/wiki/The-RobotFile-class

Both the SuiteFile and ResourceFile classes have the type attribute which indicate whether its a resource or test suite. My expectation would be that in a GeneralRule class, the RobotFile object would have a type attribute that can be inspected to find out if the object is a SuiteFile or ResourceFile.

However, when I access the type attribute, I get the following exception:

SettingTable' object has no attribute 'type'

Here's an example class.

class ImportLibraryInResource(GeneralRule):
    severity = ERROR

    def configure(self, jsonfile):
        self.jsonfn = jsonfile
        self.allowed_libs = json.load(open(jsonfile,'r'))

    def apply(self,RobotFile):
        print RobotFile.type

A test suite can be a folder, and running the "PeriodInSuiteName" does not get triggered on the a folder.

Reproducible: 100%

Steps to Reproduce:

1. Create the following directory structure

   ├── RFLintTest
   │   ├── MyLinter.Testing.robot
   │   └── SubSuite.WithPeriod
   │       ├── Test.robot
   │       └── __init__.robot
   

2. Run rflint --ignore all --error PeriodInSuiteName --recursive ./RFLintTest

   $ rflint --ignore all --error PeriodInSuiteName --recursive ./RFLintTest/
   + ./tests/RFLintTest/MyLinter.Testing.robot
   E: 0, 0: '.' in suite name 'MyLinter.Testing' (PeriodInSuiteName)
   

The --list option puts quotes around each line of output. Those are totally unnecessary.

Right now rflint only accepts filenames on the command line. It should accept directories, too, and process all robot files in that folder.

InvalidTable searchings for strings like "Test cases", "keywords", etc, but doesn't do an anchored search. Thus, a table named "Keywordz" passes since "Keyword" is part of the name.

Rflint should support -A/--argumentfile, so that you can create a file that defines which rules you want to include and exclude, which should be warnings, which should be errors, etc. The format should be the same as the robotframework argument file for consistency.

Hi
Thanks for good work!
As a RED Robot Editor (https://github.com/nokia/RED) team, we have plans to provide users with similar functionality. It seems reasonable to allow integration of lint with RED to show affected lines directly in UI.
We would like to use API for that instead of parsing text report. Do you plan to provide such function?

TagWithSpaces only looks at the [Tags] setting for a test.
It does not look at 'Force Tags' or 'Default Tags'.
It may be worth documenting that on the description of the rule.

The --verbose option should display rule documentation when running rflint. It already does this when you specify --list, but it should also do it when reporting rule violations.

If one tries to lint non-existent files, rflint returns with the code 0.

The problematic line is shown here; I believe either ERROR or "other" count should be increased so later on when reporting rflint would also fail.

Return code of zero is problematic especially in the context of CI, where automation relies on error codes to know should we continue or not. In our case for example, we use linting as a quality gate for later phases. Since we have a lot of test cases, we try to figure out in CI specifically what tests should go through the pipeline. If we have a bug in this, rflint does not stop the pipeline which becomes a problem in later phases.