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

WIP PlanemoLintBear.py: Add linter bear for planemo #1337

Open
wants to merge 1 commit 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
1 change: 1 addition & 0 deletions .ci/generate_bear_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
PINNED_PACKAGES = (
'radon',
'click',
'lxml',
)

env = Environment(loader=FileSystemLoader(THIS_DIR))
Expand Down
2 changes: 2 additions & 0 deletions bear-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ guess-language-spirit~=0.5.2
html-linter~=0.3.0
isort~=4.2
language-check~=1.0
lxml==3.6.0
munkres3~=1.0
mypy-lang~=0.4.6
nbformat~=4.1
nltk~=3.2
planemo~=0.36
proselint~=0.7.0
pycodestyle~=2.2
pydocstyle~=2.0
Expand Down
30 changes: 30 additions & 0 deletions bears/planemo/PlanemoLintBear.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason for leaving this line ?

from coalib.bearlib.abstractions.Linter import linter
from dependency_management.requirements.PipRequirement import PipRequirement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer the parenthesis like below...
from dependency_management.requirements.PipRequirement import ( PipRequirement)

from coalib.results.RESULT_SEVERITY import RESULT_SEVERITY


@linter(executable='planemo',
output_format='regex',
output_regex=r'..\s(?P<severity>\w*:)(?P<message>.*)',
severity_map={'WARNING:': RESULT_SEVERITY.MAJOR,
'CHECK:': RESULT_SEVERITY.NORMAL,
'INFO:': RESULT_SEVERITY.INFO})
class PlanemoLintBear:
"""
Checks the code with planemo lint. This will run
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is planemo?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description needs to improve IMO. A naiive user wouldn't understand much by the given description.

planemo lint over each file separately.
"""
LANGUAGES = {'galaxy'}
REQUIREMENTS = {PipRequirement('planemo', '0.36'),
PipRequirement('lxml', '3.6.0')}
AUTHORS = {'The coala developers'}
AUTHORS_EMAILS = {'[email protected]'}
ASCIINEMA_URL = 'https://asciinema.org/a/0kiduzg55d59nxhm8wuwfvl3n'
LICENSE = 'AGPL-3.0'
CAN_DETECT = {'Syntax'}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see the SEE_MORE attribute.

@staticmethod
def create_arguments(filename, file, config_file):
args = ('lint',)
return args + (filename,)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return 'lint', filename could suffice IMO.

Empty file added bears/planemo/__init__.py
Empty file.
34 changes: 34 additions & 0 deletions tests/planemo/PlanemoLintBearTest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import os
from queue import Queue
from shutil import which
from unittest.case import skipIf

from bears.planemo.PlanemoLintBear import PlanemoLintBear
from coalib.testing.LocalBearTestHelper import LocalBearTestHelper
from coalib.settings.Section import Section


@skipIf(which('planemo') is None, 'Planemo is not installed')
class PlanemoLintBearTest(LocalBearTestHelper):

def setUp(self):
self.section = Section('test section')
self.uut = PlanemoLintBear(self.section, Queue())
self.test_file = os.path.join(os.path.dirname(__file__),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason of including this in the setUp method ?

'test_files',
'planemolint_test.xml')

def test_run(self):
self.check_validity(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this checks an invalid file.

this test module also needs to run a test that is a successful valid file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see a test with the check_results method.

self.uut,
[], # Doesn't matter, planemo lint will parse the file
self.test_file,
valid=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You only test for one valid file. We wanted to improve testing, although I myself am not familiar with the details. Ask on the gitter channel about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We prefer the check_results method because we actually get to know the other attributes such as the message, the line no. etc.

self.test_file = os.path.join(os.path.dirname(__file__),
'test_files',
'valid_planemo_test.xml')
self.check_validity(
self.uut,
[], # Doesn't matter, planemo lint will parse the file
self.test_file,
valid=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're working towards writing all the future tests with check_results, so it would be nice if you could use check_results which would help us to verify the output and see if the regex is working. :)

Empty file added tests/planemo/__init__.py
Empty file.
25 changes: 25 additions & 0 deletions tests/planemo/test_files/planemolint_test.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@

<tool id="random_lines1" name="Select random lines" version="2.0.1">
<description>from a file</description>
<command interpreter="python">random_lines_two_pass.py "${input}" "${out_file1}" "${num_lines}"
#if str( $seed_source.seed_source_selector ) == "set_seed":
--seed "${seed_source.seed}"
#end if
</command>
<inputs>
<param name="num_lines" size="5" type="integer" value="1" label="Randomly select" help="lines"/>
<param format="txt" name="input" type="data" label="from"/>
<conditional name="seed_source">
<param name="seed_source_selector" type="select" label="Set a random seed">
<option value="no_seed" selected="True">Don't set seed</option>
<option value="set_seed">Set seed</option>
</param>
<when value="no_seed">
<!-- Do nothing here -->
</when>
<when value="set_seed">
<param name="seed" type="text" label="Random seed" />
</when>
</conditional>
</inputs>
</tool>
8 changes: 8 additions & 0 deletions tests/planemo/test_files/valid_planemo_test.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

<?xml version="1.0" encoding="UTF-8"?>
<note>
<to>Tove</to>
<from>Jani</from>
<heading>Reminder</heading>
<body>Don't forget me this weekend!</body>
</note>