diff --git a/rflint/parser/parser.py b/rflint/parser/parser.py index eadc936..f08e487 100644 --- a/rflint/parser/parser.py +++ b/rflint/parser/parser.py @@ -124,6 +124,7 @@ def __init__(self, path, parent=None): self.path = os.path.abspath(path) self.tables = [] self.rows = [] + self.skip_lines = [] try: self._load(path) @@ -172,10 +173,11 @@ def _load(self, path): # N.B. the caller should be catching errors self.raw_text = f.read() f.file.seek(0) + skip_pattern = re.compile(r'# *noqa\b') matcher = Matcher(re.IGNORECASE) for linenumber, raw_text in enumerate(f.readlines()): - linenumber += 1; # start counting at 1 rather than zero + linenumber += 1 # start counting at 1 rather than zero # this mimics what the robot TSV reader does -- # it replaces non-breaking spaces with regular spaces, @@ -183,6 +185,9 @@ def _load(self, path): raw_text = raw_text.replace(u'\xA0', ' ') raw_text = raw_text.rstrip() + if skip_pattern.search(raw_text): + self.skip_lines.append(linenumber) + # FIXME: I'm keeping line numbers but throwing away # where each cell starts. I should be preserving that # (though to be fair, robot is throwing that away so diff --git a/rflint/rflint.py b/rflint/rflint.py index 8355fd9..3794e2a 100644 --- a/rflint/rflint.py +++ b/rflint/rflint.py @@ -47,6 +47,7 @@ def __init__(self): # mapping of class names to instances, to enable us to # instantiate each rule exactly once self._rules = {} + self.noqa = {} for path in (builtin_rules, site_rules): for filename in glob.glob(path+"/*.py"): @@ -155,6 +156,10 @@ def _process_file(self, filename): self._print_filename = filename if self.args.print_filenames else None robot_file = RobotFactory(filename) + + for skip_line in robot_file.skip_lines: + self.noqa.setdefault(robot_file.path, []).append(skip_line) + for rule in self.general_rules: if rule.severity != IGNORE: rule.apply(robot_file) @@ -195,6 +200,10 @@ def report(self, linenumber, filename, severity, message, rulename, char): print("+ " + self._print_filename) self._print_filename = None + # do not report nor add error count if line was indicated as 'noqa' + if filename in self.noqa and linenumber in self.noqa[filename]: + return + if severity in (WARNING, ERROR): self.counts[severity] += 1 else: diff --git a/rflint/rules/testcaseRules.py b/rflint/rules/testcaseRules.py index 6a7d9df..bbb0153 100644 --- a/rflint/rules/testcaseRules.py +++ b/rflint/rules/testcaseRules.py @@ -20,12 +20,12 @@ class PeriodInTestName(TestRule): '''Warn about periods in the testcase name - + Since robot uses "." as a path separator, using a "." in a testcase - name can lead to ambiguity. + name can lead to ambiguity. ''' severity = WARNING - + def apply(self,testcase): if "." in testcase.name: self.report(testcase, "'.' in testcase name '%s'" % testcase.name, testcase.linenumber) @@ -35,9 +35,16 @@ class TagWithSpaces(TestRule): severity=ERROR def apply(self, testcase): + tags_with_spaces = [] for tag in testcase.tags: if ((" " in tag) or ("\t" in tag)): - self.report(testcase, "space not allowed in tag name: '%s'" % tag, testcase.linenumber) + tags_with_spaces.append(tag) + # second loop is to discover line number of tag appearance + for setting in testcase.settings: + for cell in setting: + for tag_with_spaces in tags_with_spaces: + if tag_with_spaces in cell: + self.report(testcase, "space not allowed in tag name: '%s'" % tag_with_spaces, setting.startline) class RequireTestDocumentation(TestRule): '''Verify that a test suite has documentation diff --git a/test_data/acceptance/noqa.robot b/test_data/acceptance/noqa.robot new file mode 100644 index 0000000..ecca514 --- /dev/null +++ b/test_data/acceptance/noqa.robot @@ -0,0 +1,40 @@ +*** Settings *** +Documentation Test cases with skip line indicator (# noqa) + + +*** Test Cases *** +Skip Tag With Spaces + [Documentation] Skip next line containing tag with spaces + [Tags] Tag With Spaces TagWithNoSpace # noqa + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Do Not Skip Tag With Spaces + [Documentation] Report tag with spaces + [Tags] Tag With Spaces TagWithNoSpace + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Skip This Test Case With Dot. #noqa + [Documentation] Do not report improper test case name + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Do Not Skip This Test Case With Dot. + [Documentation] Report dot presence in test case name + ${foo} Set Variable some variable + ${bar} Set Variable another variable + +Skip Very Long Line + [Documentation] Do not report very long line (+100 chars) + ${long_variable} Set Variable This is a very long sentence with too many characters that should not be reported because of the 'noqa' comment at the end of this line # noqa and something else + ${short_variable} Set Variable This is a short variable + +Do Not Skip Very Long Line + [Documentation] Report very long line (+100 chars) + ${long_variable} Set Variable This is a very long sentence with too many characters that will be reported because of lack of 'noqa' comment at the end of this line + ${short_variable} Set Variable This is a short variable + +Do Not Skip Line Because Of Improper Comment # noqaa + [Documentation] Improper comment should not make rflint omit the line + ${var} Set Variable This test case contains only one test step diff --git a/tests/acceptance/noqa.robot b/tests/acceptance/noqa.robot new file mode 100644 index 0000000..480d3ea --- /dev/null +++ b/tests/acceptance/noqa.robot @@ -0,0 +1,29 @@ +*** Settings *** +| Documentation +| ... | Runs rflint against the rflint test suites and resource files +| +| Library | OperatingSystem +| Library | Process +| Resource | SharedKeywords.robot + +*** Test Cases *** +| Run rflint and check if output contains given warnings and errors +| | Run rf-lint with the following options: +| | ... | test_data/acceptance/noqa.robot +| | +| | @{messages}= | Split to lines | ${result.stdout} +| | ${warnings}= | Get match count | ${messages} | regexp=^W: +| | ${errors}= | Get match count | ${messages} | regexp=^E: +| | +| | Run keyword if | "${result.rc}" != "${1}" or ${warnings} != 3 or ${errors} != 1 +| | ... | Fail | unexpectected errors or warnings: \n${result.stdout}\n${result.stderr} +| | +| | Output should contain +| | ... | W: 35, 100: Line is too long (exceeds 100 characters) (LineTooLong) +| | ... | E: 14, 0: space not allowed in tag name: 'Tag With Spaces' (TagWithSpaces) +| | ... | W: 23, 0: '.' in testcase name 'Do Not Skip This Test Case With Dot.' (PeriodInTestName) +| | ... | W: 38, 0: Too few steps (1) in test case (TooFewTestSteps) +| | Output should not contain +| | ... | W: 30, 100: Line is too long (exceeds 100 characters) (LineTooLong) +| | ... | E: 8, 0: space not allowed in tag name: 'Tag With Spaces' (TagWithSpaces) +| | ... | W: 18, 0: '.' in testcase name 'Skip This Test Case With Dot.' (PeriodInTestName)