Skip to content

Skip lines with #noqa comment #81

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion rflint/parser/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -172,17 +173,21 @@ 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,
# and then strips trailing whitespace
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
Expand Down
9 changes: 9 additions & 0 deletions rflint/rflint.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand Down
15 changes: 11 additions & 4 deletions rflint/rules/testcaseRules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
40 changes: 40 additions & 0 deletions test_data/acceptance/noqa.robot
Original file line number Diff line number Diff line change
@@ -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
29 changes: 29 additions & 0 deletions tests/acceptance/noqa.robot
Original file line number Diff line number Diff line change
@@ -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)