Skip to content

Commit

Permalink
Merge pull request #2959 from mashehu/handle-request-errors
Browse files Browse the repository at this point in the history
Linting: handle request errors more gracefully for actions validation
  • Loading branch information
mashehu authored May 8, 2024
2 parents b56906e + bda8d42 commit 293c781
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- Include test for presence of versions in snapshot ([#2888](https://github.com/nf-core/tools/pull/2888))
- Components: set correct sha before running component lint tests ([#2952](https://github.com/nf-core/tools/pull/2952))
- Less strict logo comparison ([#2956](https://github.com/nf-core/tools/pull/2956))
- Handle request errors more gracefully for actions validation ([#2959](https://github.com/nf-core/tools/pull/2959))

### Download

Expand Down
20 changes: 14 additions & 6 deletions nf_core/lint/actions_schema_validation.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import glob
import logging
import os
from typing import Any, Dict, List

import jsonschema
import requests
import yaml


def actions_schema_validation(self):
def actions_schema_validation(self) -> Dict[str, List[str]]:
"""Checks that the GitHub Action workflow yml/yaml files adhere to the correct schema
nf-core pipelines use GitHub actions workflows to run CI tests, check formatting and also linting, among others.
Expand All @@ -17,8 +18,9 @@ def actions_schema_validation(self):
To pass this test, make sure that all your workflows contain the required properties ``on`` and ``jobs`` and that
all other properties are of the correct type, as specified in the schema (link above).
"""
passed = []
failed = []
passed: List[str] = []
failed: List[str] = []
warned: List[str] = []

# Only show error messages from schema
logging.getLogger("nf_core.schema").setLevel(logging.ERROR)
Expand All @@ -28,7 +30,13 @@ def actions_schema_validation(self):

# Load the GitHub workflow schema
r = requests.get("https://json.schemastore.org/github-workflow", allow_redirects=True)
schema = r.json()
# handle "Service Unavailable" error
if r.status_code not in [200, 301]:
warned.append(
f"Failed to fetch schema: Response code for `https://json.schemastore.org/github-workflow` was {r.status_code}"
)
return {"passed": passed, "failed": failed, "warned": warned}
schema: Dict[str, Any] = r.json()

# Validate all workflows against the schema
for wf_path in action_workflows:
Expand All @@ -46,7 +54,7 @@ def actions_schema_validation(self):
try:
wf_json["on"] = wf_json.pop(True)
except Exception:
failed.append("Missing 'on' keyword in {}.format(wf)")
failed.append(f"Missing 'on' keyword in {wf}")

# Validate the workflow
try:
Expand All @@ -55,4 +63,4 @@ def actions_schema_validation(self):
except Exception as e:
failed.append(f"Workflow validation failed for {wf}: {e}")

return {"passed": passed, "failed": failed}
return {"passed": passed, "failed": failed, "warned": warned}
19 changes: 11 additions & 8 deletions tests/lint/actions_schema_validation.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import os
from pathlib import Path

import yaml

Expand All @@ -9,10 +9,11 @@ def test_actions_schema_validation_missing_jobs(self):
"""Missing 'jobs' field should result in failure"""
new_pipeline = self._make_pipeline_copy()

with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml")) as fh:
awstest_yml_path = Path(new_pipeline) / ".github" / "workflows" / "awstest.yml"
with open(awstest_yml_path) as fh:
awstest_yml = yaml.safe_load(fh)
awstest_yml.pop("jobs")
with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml"), "w") as fh:
with open(awstest_yml_path, "w") as fh:
yaml.dump(awstest_yml, fh)

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
Expand All @@ -27,29 +28,31 @@ def test_actions_schema_validation_missing_on(self):
"""Missing 'on' field should result in failure"""
new_pipeline = self._make_pipeline_copy()

with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml")) as fh:
awstest_yml_path = Path(new_pipeline) / ".github" / "workflows" / "awstest.yml"
with open(awstest_yml_path) as fh:
awstest_yml = yaml.safe_load(fh)
awstest_yml.pop(True)
with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml"), "w") as fh:
with open(awstest_yml_path, "w") as fh:
yaml.dump(awstest_yml, fh)

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
lint_obj._load()

results = lint_obj.actions_schema_validation()

assert results["failed"][0] == "Missing 'on' keyword in {}.format(wf)"
assert results["failed"][0] == "Missing 'on' keyword in awstest.yml"
assert "Workflow validation failed for awstest.yml: 'on' is a required property" in results["failed"][1]


def test_actions_schema_validation_fails_for_additional_property(self):
"""Missing 'jobs' field should result in failure"""
new_pipeline = self._make_pipeline_copy()

with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml")) as fh:
awstest_yml_path = Path(new_pipeline) / ".github" / "workflows" / "awstest.yml"
with open(awstest_yml_path) as fh:
awstest_yml = yaml.safe_load(fh)
awstest_yml["not_jobs"] = awstest_yml["jobs"]
with open(os.path.join(new_pipeline, ".github", "workflows", "awstest.yml"), "w") as fh:
with open(awstest_yml_path, "w") as fh:
yaml.dump(awstest_yml, fh)

lint_obj = nf_core.lint.PipelineLint(new_pipeline)
Expand Down

0 comments on commit 293c781

Please sign in to comment.