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

Make SARIF output specification compliant #2668

Merged
merged 4 commits into from
Nov 17, 2022

Conversation

atiterlea
Copy link
Contributor

@atiterlea atiterlea commented Nov 9, 2022

The implementation uses the (unique) tag value instead of the rule ID.

Fixes: #2667

@atiterlea atiterlea requested a review from a team as a code owner November 9, 2022 20:29
@atiterlea
Copy link
Contributor Author

This failed test doesn't appear to have been introduced by my changes as it's failing in other PRs and on the mainline branch as well:

=================================== FAILURES ===================================
______________ test_template_lookup[template_lookup_missing-True] ______________
[gw0] linux -- Python 3.8.14 /home/runner/work/ansible-lint/ansible-lint/.tox/py38/bin/python

role = 'template_lookup_missing', expect_warning = True

    @pytest.mark.parametrize(
        ("role", "expect_warning"),
        (
            ("template_lookup", False),
            # With 2.15 ansible replaced the runtime Warning about inability to
            # open a file in file lookup with a full error.
            ("template_lookup_missing", runtime.version_in_range(upper="2.14")),
        ),
    )
    def test_template_lookup(role: str, expect_warning: bool) -> None:
        """Assure lookup plugins used in templates does not trigger Ansible warnings."""
        task_path = os.path.realpath(
            os.path.join(
                os.path.dirname(os.path.realpath(__file__)),
                "..",
                "examples",
                "roles",
                role,
                "tasks",
                "main.yml",
            )
        )
        result = run_ansible_lint("-v", task_path)
>       assert ("Unable to find" in result.stderr) == expect_warning
E       assert ('Unable to find' in 'WARNING  Collection install skipped because ansible versions before 2.14 do not support an offline mode.\nINFO     Se...  1 name[missing]  basic   idiom                \n\nFailed after min profile: 3 failure(s), 0 warning(s) on 1 files.\n') == True
E        +  where 'WARNING  Collection install skipped because ansible versions before 2.14 do not support an offline mode.\nINFO     Se...  1 name[missing]  basic   idiom                \n\nFailed after min profile: 3 failure(s), 0 warning(s) on 1 files.\n' = CompletedProcess(args=['/home/runner/work/ansible-lint/ansible-lint/.tox/py38/bin/python', '-m', 'ansiblelint', '--off... 1 name[missing]  basic   idiom                \n\nFailed after min profile: 3 failure(s), 0 warning(s) on 1 files.\n").stderr

expect_warning = True
result     = CompletedProcess(args=['/home/runner/work/ansible-lint/ansible-lint/.tox/py38/bin/python', '-m', 'ansiblelint', '--off... 1 name[missing]  basic   idiom                \n\nFailed after min profile: 3 failure(s), 0 warning(s) on 1 files.\n")
role       = 'template_lookup_missing'
task_path  = '/home/runner/work/ansible-lint/ansible-lint/examples/roles/template_lookup_missing/tasks/main.yml'

test/test_utils.py:2[76](https://github.com/ansible/ansible-lint/actions/runs/3418307509/jobs/5690444320#step:11:77): AssertionError

@ssbarnea
Copy link
Member

You are right. Someone needs to fix that test before we can merge the other PRs. I am currently in vacation for another week so I am not sure if I will find time to do it myself.

@ssbarnea ssbarnea added the bug label Nov 10, 2022
@atiterlea
Copy link
Contributor Author

The test that's failing in the py39 (wsl) check looks like it's tied to some dependency mismatch between the INI module offered in Linux distros and the module loaded into a Windows filesystem. Is it possible that the module just isn't loaded on WSL, or not found in its expected location?

@ssbarnea ssbarnea changed the title Fix for issue 2667 - SARIF output specification Make SARIF output specification compliant Nov 17, 2022
@ssbarnea ssbarnea merged commit cef6a7f into ansible:main Nov 17, 2022
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.

SARIF output does not adhere to specification
3 participants