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 sure everything passes the linters, and enable linting via the makefile #681

Merged
merged 3 commits into from
Aug 19, 2024
Merged
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
22 changes: 8 additions & 14 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ repos:
hooks:
- id: flake8
name: flake8 lint
args: [
'--ignore=E203,E266,W503,F403',
'--exclude=".git, .mypy_cache, .pytest_cache, build, dist"',
'--max-line-length=120',
'--max-complexity=18',
'--select="B,C,E,F,W,T4,B9"'
]
args:
[
"--ignore=E203,E266,W503,F403",
'--exclude=".git, .mypy_cache, .pytest_cache, build, dist"',
"--max-line-length=120",
"--max-complexity=18",
'--select="B,C,E,F,W,T4,B9"',
]
additional_dependencies:
- flake8-broken-line
- flake8-bugbear
Expand Down Expand Up @@ -56,10 +57,3 @@ repos:
hooks:
- id: sync_with_poetry
args: []

- repo: https://github.com/rtts/djhtml
murdo-moj marked this conversation as resolved.
Show resolved Hide resolved
rev: "3.0.6" # replace with the latest tag on GitHub
hooks:
- id: djhtml
- id: djcss
- id: djjs
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ run:
poetry run python manage.py runserver

# Run unit tests
test: unit integration
test: unit integration lint

# Run Python unit tests
unit:
Expand All @@ -78,4 +78,7 @@ clean:
rm -f $(ENV_FILE)
find . -name "*.pyc" -exec rm -f {} \;

.PHONY: all build install_deps set_env collect_static migrate setup_waffle_switches messages compilemessages run test unit integration clean
lint:
pre-commit run --all-files

.PHONY: all build install_deps set_env collect_static migrate setup_waffle_switches messages compilemessages run test unit integration clean lint
3 changes: 0 additions & 3 deletions feedback/admin.py

This file was deleted.

8 changes: 4 additions & 4 deletions feedback/templates/thanks.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
{% load static %}

{% block content %}
<div class="govuk-grid-row">
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<h1 class="govuk-heading-xl">
{% translate 'Thank you for your feedback' %}
</h1>

<p class="govuk-body">{% translate 'Your feedback will help us improve the service.' %}</p>
</div>
</div>

</div>
{% endblock content %}
</div>
{% endblock content %}
3 changes: 0 additions & 3 deletions feedback/tests.py

This file was deleted.

2 changes: 1 addition & 1 deletion home/templatetags/snippets.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def _align_snippet(snippet, max_chars):
Align a snippet to a few words before start_mark_idx
"""
start_mark_idx = snippet.find("<mark>")
end_mark_idx = snippet[start_mark_idx + 1:].find("</mark>")
end_mark_idx = snippet[start_mark_idx + 1 :].find("</mark>")

# If the mark is at the beginning of the first remaining paragraph,
# no further alignment is required.
Expand Down
3 changes: 0 additions & 3 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ const config = {
// Stop running tests after `n` failures
// bail: 0,

// The directory where Jest should store its cached dependency information
// cacheDirectory: "/private/var/folders/lh/0dtbp3254mvc5pvhcx4ngnf80000gq/T/jest_dz",

// Automatically clear mock calls, instances, contexts and results before every test
clearMocks: true,

Expand Down
22 changes: 14 additions & 8 deletions lib/datahub-client/data_platform_catalogue/client/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@
import logging
from typing import Any, Sequence, Tuple

from datahub.configuration.common import GraphError # pylint: disable=E0611
from datahub.ingestion.graph.client import DataHubGraph # pylint: disable=E0611

from data_platform_catalogue.client.exceptions import CatalogueError
from data_platform_catalogue.client.graphql_helpers import (
get_graphql_query,
Expand All @@ -28,6 +25,8 @@
SearchResult,
SortOption,
)
from datahub.configuration.common import GraphError # pylint: disable=E0611
from datahub.ingestion.graph.client import DataHubGraph # pylint: disable=E0611

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -115,24 +114,31 @@ def _parse_search_results(self, response) -> Tuple[list, list]:

try:
if entity_type == "DATASET":
parsed_result = self._parse_dataset(entity, matched_fields, ResultType.TABLE)
parsed_result = self._parse_dataset(
entity, matched_fields, ResultType.TABLE
)
page_results.append(parsed_result)
elif entity_type == "CHART":
parsed_result = self._parse_dataset(entity, matched_fields, ResultType.CHART)
parsed_result = self._parse_dataset(
entity, matched_fields, ResultType.CHART
)
page_results.append(parsed_result)
elif entity_type == "CONTAINER":
parsed_result = self._parse_container(entity, matched_fields, ResultType.DATABASE)
parsed_result = self._parse_container(
entity, matched_fields, ResultType.DATABASE
)
page_results.append(parsed_result)
elif entity_type == "DASHBOARD":
parsed_result = self._parse_container(entity, matched_fields, ResultType.DASHBOARD)
parsed_result = self._parse_container(
entity, matched_fields, ResultType.DASHBOARD
)
page_results.append(parsed_result)
else:
raise Exception
except Exception:
logger.warn(f"Parsing for result {entity_urn} failed")
malformed_result_urns.append(entity_urn)


return page_results, malformed_result_urns

@staticmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from unittest.mock import MagicMock, patch

import pytest

from data_platform_catalogue.client.datahub_client import (
DataHubCatalogueClient,
InvalidDomain,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from datetime import datetime, timezone

import pytest

from data_platform_catalogue.client.graphql_helpers import (
_make_user_email_from_urn,
parse_columns,
Expand Down
9 changes: 6 additions & 3 deletions lib/datahub-client/tests/client/datahub/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ def test_dataset_result(mock_graph, searcher):
)
assert response == expected


def test_bad_entity_type(mock_graph, searcher):
datahub_response = {
"searchAcrossEntities": {
Expand Down Expand Up @@ -275,7 +276,9 @@ def test_bad_entity_type(mock_graph, searcher):
expected = expected = SearchResponse(
total_results=1,
page_results=[],
malformed_result_urns=["urn:li:dataset:(urn:li:dataPlatform:bigquery,calm-pagoda-323403.jaffle_shop.customers,PROD)"],
malformed_result_urns=[
"urn:li:dataset:(urn:li:dataPlatform:bigquery,calm-pagoda-323403.jaffle_shop.customers,PROD)"
],
facets=SearchFacets(facets={}),
)
assert response == expected
Expand Down Expand Up @@ -344,9 +347,9 @@ def test_2_dataset_results_with_one_malformed_result(mock_graph, searcher):
"value": "moj-reg-prod-hmpps-assess-risks-and-needs-prod-glue-job",
},
],
}
},
},
}
},
],
}
}
Expand Down
99 changes: 42 additions & 57 deletions lib/datahub-client/tests/test_helpers/mce_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
load_json_file,
)
from datahub.utilities.urns.urn import Urn

from tests.test_helpers.type_helpers import PytestConfig

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -154,20 +155,16 @@ def get_entity_urns(events_file: str) -> Set[str]:
def _get_entity_urns(events_list: List[Dict]) -> Set[str]:
entity_type = "dataset"
# mce urns
mce_urns = set(
[
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in events_list
if _get_filter(mce=True, entity_type=entity_type)(x)
]
)
mcp_urns = set(
[
_get_element(x, _get_mcp_urn_path_spec())
for x in events_list
if _get_filter(mcp=True, entity_type=entity_type)(x)
]
)
mce_urns = {
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in events_list
if _get_filter(mce=True, entity_type=entity_type)(x)
}
mcp_urns = {
_get_element(x, _get_mcp_urn_path_spec())
for x in events_list
if _get_filter(mcp=True, entity_type=entity_type)(x)
}
all_urns = mce_urns.union(mcp_urns)
return all_urns

Expand Down Expand Up @@ -250,24 +247,20 @@ def assert_for_each_entity(
test_output = load_json_file(file)
assert isinstance(test_output, list)
# mce urns
mce_urns = set(
[
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in test_output
if _get_filter(mce=True, entity_type=entity_type)(x)
]
)
mcp_urns = set(
[
_get_element(x, _get_mcp_urn_path_spec())
for x in test_output
if _get_filter(mcp=True, entity_type=entity_type)(x)
]
)
mce_urns = {
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in test_output
if _get_filter(mce=True, entity_type=entity_type)(x)
}
mcp_urns = {
_get_element(x, _get_mcp_urn_path_spec())
for x in test_output
if _get_filter(mcp=True, entity_type=entity_type)(x)
}
all_urns = mce_urns.union(mcp_urns)
# there should not be any None urns
assert None not in all_urns
aspect_map = {urn: None for urn in all_urns}
aspect_map = dict.fromkeys(all_urns)
# iterate over all mcps
for o in [
mcp
Expand Down Expand Up @@ -362,20 +355,16 @@ def assert_entity_urn_not_like(entity_type: str, regex_pattern: str, file: str)
test_output = load_json_file(file)
assert isinstance(test_output, list)
# mce urns
mce_urns = set(
[
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in test_output
if _get_filter(mce=True, entity_type=entity_type)(x)
]
)
mcp_urns = set(
[
_get_element(x, _get_mcp_urn_path_spec())
for x in test_output
if _get_filter(mcp=True, entity_type=entity_type)(x)
]
)
mce_urns = {
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in test_output
if _get_filter(mce=True, entity_type=entity_type)(x)
}
mcp_urns = {
_get_element(x, _get_mcp_urn_path_spec())
for x in test_output
if _get_filter(mcp=True, entity_type=entity_type)(x)
}
all_urns = mce_urns.union(mcp_urns)
print(all_urns)
matched_urns = [u for u in all_urns if re.match(regex_pattern, u)]
Expand All @@ -392,20 +381,16 @@ def assert_entity_urn_like(entity_type: str, regex_pattern: str, file: str) -> i
test_output = load_json_file(file)
assert isinstance(test_output, list)
# mce urns
mce_urns = set(
[
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in test_output
if _get_filter(mce=True, entity_type=entity_type)(x)
]
)
mcp_urns = set(
[
_get_element(x, _get_mcp_urn_path_spec())
for x in test_output
if _get_filter(mcp=True, entity_type=entity_type)(x)
]
)
mce_urns = {
_get_element(x, _get_mce_urn_path_spec(entity_type))
for x in test_output
if _get_filter(mce=True, entity_type=entity_type)(x)
}
mcp_urns = {
_get_element(x, _get_mcp_urn_path_spec())
for x in test_output
if _get_filter(mcp=True, entity_type=entity_type)(x)
}
all_urns = mce_urns.union(mcp_urns)
print(all_urns)
matched_urns = [u for u in all_urns if re.match(regex_pattern, u)]
Expand Down
2 changes: 1 addition & 1 deletion scss/_details.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
padding: govuk-spacing(3) govuk-spacing(4);
margin-bottom: govuk-spacing(6);
overflow-wrap: break-word;
}
}
2 changes: 1 addition & 1 deletion scss/_glossary.scss
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@
overflow-y: auto;
margin-bottom: govuk-spacing(6);
}
}
}
2 changes: 1 addition & 1 deletion static/assets/js/enhanced-glossary.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const updateResults = () => {
const terms = Array.from(el.querySelectorAll(".term"));
const isEmpty =
terms.length === 0 ||
terms.every((term) => term.classList.contains("govuk-!-display-none"));
terms.every((term) => term.classList.contains("govuk-!-display-none"));

if (isEmpty) {
el.classList.add("govuk-!-display-none");
Expand Down
6 changes: 3 additions & 3 deletions templates/base/footer.html
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ <h2 class="govuk-visually-hidden">{% translate "Support links" %}</h2>
</div>
<div class="govuk-footer__meta-item">
{% blocktranslate trimmed with href="https://www.nationalarchives.gov.uk/information-management/re-using-public-sector-information/uk-government-licensing-framework/crown-copyright/" %}
<a
class="govuk-footer__link govuk-footer__copyright-logo"
href="{{ href }}">© Crown copyright</a>
<a
class="govuk-footer__link govuk-footer__copyright-logo"
href="{{ href }}">© Crown copyright</a>
{% endblocktranslate %}
</div>
</div>
Expand Down
6 changes: 3 additions & 3 deletions tests/home/templatetags/test_snippets.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
(
"a" * 200 + "\n\n" + "word word word " * 100 + "hello <mark>world</mark>",
200,
"…word word word word word word word word word word word word word word word word word word word word word word word hello <mark>world</mark>",
"…word word word word word word word word word word word word word word word word word word word word word word word hello <mark>world</mark>", # noqa: E501
),
(
"A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or remand, recall notices not acted upon, sentence miscalculation or discharging the wrong person on escort.",
"A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or remand, recall notices not acted upon, sentence miscalculation or discharging the wrong person on escort.", # noqa: E501
300,
"A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or…",
"A prisoner is released in error if they are wrongly discharged from a prison or court when they should have remained in custody, where the prisoner has not deliberately played a part in the error (i.e. the prisoner had no intent of <mark>escaping</mark>).\n\nExamples include misplaced warrants for imprisonment or…", # noqa: E501
),
],
ids=(
Expand Down
1 change: 0 additions & 1 deletion tests/integration/test_primary_nav.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,3 @@ def start_on_the_home_page(self):

assert heading_text == "Find MoJ data"
assert self.selenium.title.split("-")[0].strip() == "Home"

3 changes: 0 additions & 3 deletions users/admin.py

This file was deleted.

Loading
Loading