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

Add codeclimate formatter (json) #1308

Merged
merged 5 commits into from Feb 7, 2021

Conversation

thushjandan
Copy link
Contributor

I have introduced a new parameter called --parseable-codeclimate. This flag will indicate ansible-lint to use the new formatter CodeclimateJSONFormatter. It will export the matcherror results in JSON following the Codeclimate spec. To test this formatter I have created few additional unit test cases. The formatter expects a list of MatchError objects instead of a single MatchError Object.

Example output with the existing formatter ParseableSeverityFormatter

vagrant@ubuntu-focal:/vagrant/src$ python3 -m ansiblelint --parseable-severity hello.yml
Added ANSIBLE_LIBRARY=.cache/modules
Added ANSIBLE_ROLES_PATH=.cache/roles
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: hello.yml
WARNING  Listing 6 violation(s) that are fatal
hello.yml:7: [YAML] [VERY_LOW] too many blank lines (3 > 2)
hello.yml:8: [YAML] [VERY_LOW] trailing spaces
hello.yml:11: [206] [LOW] Variables should have spaces before and after: {{bla}}
hello.yml:13: [206] [LOW] Variables should have spaces before and after: {{bla}}
hello.yml:13: [301] [HIGH] Commands should not change things if nothing needs doing
hello.yml:13: [502] [MEDIUM] All tasks should be named
You can skip specific rules or tags by adding them to your configuration file:
# .ansible-lint
warn_list:  # or 'skip_list' to silence them completely
  - '206'  # Variables should have spaces before and after:  {{ var_name }}
  - '301'  # Commands should not change things if nothing needs doing
  - '502'  # All tasks should be named
  - 'YAML'  # Violations reported by yamllint
  - experimental  # all rules tagged as experimental
Finished with 6 failure(s), 0 warning(s) on 1 files.

Example output with the new formatter CodeclimateJSONFormatter.

vagrant@ubuntu-focal:/vagrant/src$ python3 -m ansiblelint --parseable-codeclimate hello.yml
Added ANSIBLE_LIBRARY=.cache/modules
Added ANSIBLE_ROLES_PATH=.cache/roles
WARNING  Overriding detected file kind 'yaml' with 'playbook' for given positional argument: hello.yml
[{"type": "issue", "check_name": "[YAML] too many blank lines (3 > 2)", "categories": ["formatting", "experimental", "yamllint"], "severity": "info", "description": "Rule violations reported by YamlLint when this is installed.\n\nYou can fully disable all of them by adding 'YAML' to the 'skip_list'.\n\nSpecific tag identifiers that are printed at the end of rule name,\nlike 'trailing-spaces' or 'indentation' can also be be skipped, allowing\nyou to have a more fine control.\n", "fingerprint": "b0851e41d26992bf39ebd1d1a87f8249f4894aba363f9e910feae4d1b3085115", "location": {"path": "hello.yml", "lines": {"begin": 7}}}, {"type": "issue", "check_name": "[YAML] trailing spaces", "categories": ["formatting", "experimental", "yamllint"], "severity": "info", "description": "Rule violations reported by YamlLint when this is installed.\n\nYou can fully disable all of them by adding 'YAML' to the 'skip_list'.\n\nSpecific tag identifiers that are printed at the end of rule name,\nlike 'trailing-spaces' or 'indentation' can also be be skipped, allowing\nyou to have a more fine control.\n", "fingerprint": "e65977880f088335e900ec47400d8ae03436c18bb7066c011ac64a07b9058e34", "location": {"path": "hello.yml", "lines": {"begin": 8}}}, {"type": "issue", "check_name": "[206] Variables should have spaces before and after: {{bla}}", "categories": ["formatting"], "severity": "minor", "description": "Variables should have spaces before and after: ``{{ var_name }}``", "fingerprint": "1b0c8063e5bd49994fea26f25dc96286b44542884f8e3324e5347fb29cd1fdae", "location": {"path": "hello.yml", "lines": {"begin": 11}}, "content": {"body": "Task/Handler: debug msg={{bla}}"}}, {"type": "issue", "check_name": "[206] Variables should have spaces before and after: {{bla}}", "categories": ["formatting"], "severity": "minor", "description": "Variables should have spaces before and after: ``{{ var_name }}``", "fingerprint": "81fec0f931a0895bca0141a9137fe37c71700b5b81baf663d0bdda38d593b4ce", "location": {"path": "hello.yml", "lines": {"begin": 13}}, "content": {"body": "Task/Handler: command  echo {{bla}}"}}, {"type": "issue", "check_name": "[301] Commands should not change things if nothing needs doing", "categories": ["command-shell", "idempotency"], "severity": "critical", "description": "Commands should either read information (and thus set ``changed_when``) or not do something if it has already been done (using creates/removes) or only do it if another check has a particular result (``when``)", "fingerprint": "d57fbb328371a4c6ae1d54141df55f1b778aaaef8da75b13e2fdd73b3cda5dae", "location": {"path": "hello.yml", "lines": {"begin": 13}}, "content": {"body": "Task/Handler: command  echo {{bla}}"}}, {"type": "issue", "check_name": "[502] All tasks should be named", "categories": ["task", "readability"], "severity": "major", "description": "All tasks should have a distinct name for readability and for ``--start-at-task`` to work", "fingerprint": "943dffb61e26499b71e10a95197833b159e48bfc6eebef8f23adaf9b37eafc0d", "location": {"path": "hello.yml", "lines": {"begin": 13}}, "content": {"body": "Task/Handler: command  echo {{bla}}"}}]
You can skip specific rules or tags by adding them to your configuration file:
# .ansible-lint
warn_list:  # or 'skip_list' to silence them completely
  - '206'  # Variables should have spaces before and after:  {{ var_name }}
  - '301'  # Commands should not change things if nothing needs doing
  - '502'  # All tasks should be named
  - 'YAML'  # Violations reported by yamllint
  - experimental  # all rules tagged as experimental
Finished with 6 failure(s), 0 warning(s) on 1 files.

@thushjandan thushjandan marked this pull request as draft February 6, 2021 11:35
parser.add_argument('--parseable-codeclimate', dest='parseable_codeclimate',
default=False,
action='store_true',
help="parseable output in the format of codeclimate json report")
Copy link
Member

Choose a reason for hiding this comment

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

Parseable severity was a historical mistake and I do not want to clutter the CLI options with very specific cases.

Instead you could add a new formatter and extend -f {rich,plain,rst} to add codeclimate to the list of allowed options, without adding new command line options.

# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.
# pylint: disable=preferred-module # FIXME: remove once migrated per GH-725
Copy link
Member

Choose a reason for hiding this comment

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

Remove the notice is not needed for new files.

from ansiblelint.rules import AnsibleLintRule


class TestCodeclimateJSONFormatter(unittest.TestCase):
Copy link
Member

Choose a reason for hiding this comment

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

You are missing the most important test: explicit test of output produced by an file. Feel free to reuse one of the examples and add a check that validates that the JSON output is exactly the one we expect and know to
be accepted by codeclimate. Without this we risk of breaking this feature in the future.

Pick one small but relevant example for that, just enough to validate the output.

@ssbarnea
Copy link
Member

ssbarnea commented Feb 6, 2021

Do not forget to rebase on master as I just merged a code formatting change that will cause some problems.

If you can address my comments I will include your patch in v5.0 release, which I plan to make tomorrow.

@thushjandan
Copy link
Contributor Author

Thanks for the review. I will implement your suggestions today.

@thushjandan thushjandan marked this pull request as ready for review February 6, 2021 17:10
@thushjandan
Copy link
Contributor Author

Hi,
I have implemented the proposed changes. Could you please review my PR again?

Copy link
Member

@ssbarnea ssbarnea left a comment

Choose a reason for hiding this comment

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

Thanks, it looks good now. I hope to see more pull-requests from you. If you could help me fix one of the open bugs it would be awesome.

I would try to avoid feature requests as we would never run of ideas, still bugs affect our users more than missing features.

@ssbarnea ssbarnea changed the title Additional Formatter to export results as codeclimate json Add codeclimate formatter (json) Feb 7, 2021
@ssbarnea ssbarnea merged commit ac68e55 into ansible:master Feb 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants