Skip to content

Commit

Permalink
chore: add dedicated Python CI + ruff (#274)
Browse files Browse the repository at this point in the history
<!-- Please use this template for your pull request. -->
<!-- Please use the sections that you need and delete other sections -->

## This PR
<!-- add the description of the PR here -->

- added a dedicated Python CI workflow
- added ruff for linting (only dependency sorting is enabled for now)
and formatting
- added a couple of missing type hints, enabling mypy comes in a
separate PR

Signed-off-by: gruebel <anton.gruebel@gmail.com>
  • Loading branch information
gruebel authored Oct 4, 2024
1 parent 807c0d2 commit df1f62e
Show file tree
Hide file tree
Showing 5 changed files with 103 additions and 43 deletions.
32 changes: 32 additions & 0 deletions .github/workflows/pr-python.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: PR Python checks

on:
pull_request:
branches:
- main

permissions:
contents: read

env:
WORKING_DIR: tools/repo_parser

jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
with:
python-version: 3.12

- name: Install dependencies
working-directory: ${{ env.WORKING_DIR }}
run: pip install -r requirements.txt

- name: Format
working-directory: ${{ env.WORKING_DIR }}
run: ruff format --check
- name: Lint
working-directory: ${{ env.WORKING_DIR }}
run: ruff check
11 changes: 11 additions & 0 deletions tools/repo_parser/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[tool.ruff]
line-length = 120
target-version = "py312"

[tool.ruff.lint]
select = [
"I",
]

[tool.ruff.format]
quote-style = "single"
1 change: 1 addition & 0 deletions tools/repo_parser/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mypy
pytest
pytest-cov
ruff
84 changes: 47 additions & 37 deletions tools/repo_parser/spec_finder.py
Original file line number Diff line number Diff line change
@@ -1,28 +1,32 @@
#!/usr/bin/python
import urllib.request
from __future__ import annotations

import configparser
import json
import re
import difflib
import os
import re
import sys
import configparser
from typing import TypedDict, Optional, cast
import urllib.request
from typing import Any, TypedDict, cast


class Config(TypedDict):
file_extension: str
multiline_regex: Optional[str]
number_subregex: Optional[str]
text_subregex: Optional[str]
inline_comment_prefix: Optional[str]
multiline_regex: str | None
number_subregex: str | None
text_subregex: str | None
inline_comment_prefix: str | None

def _demarkdown(t):

def _demarkdown(t: str) -> str:
return t.replace('**', '').replace('`', '').replace('"', '')

def get_spec_parser(code_dir) -> Config:

def get_spec_parser(code_dir: str) -> Config:
with open(os.path.join(code_dir, '.specrc')) as f:
data = '\n'.join(f.readlines())

typical = configparser.ConfigParser(comment_prefixes=None)
typical = configparser.ConfigParser(comment_prefixes=())
typical.read_string(data)
retval = typical['spec']

Expand All @@ -40,17 +44,18 @@ def get_spec_parser(code_dir) -> Config:
return cast(Config, retval)



def get_spec(force_refresh=False, path_prefix="./"):
def get_spec(force_refresh: bool = False, path_prefix: str = './') -> dict[str, Any]:
spec_path = os.path.join(path_prefix, 'specification.json')
print("Going to look in ", spec_path)
data = ""
print('Going to look in ', spec_path)
data = ''
if os.path.exists(spec_path) and not force_refresh:
with open(spec_path) as f:
data = ''.join(f.readlines())
else:
# TODO: Status code check
spec_response = urllib.request.urlopen('https://raw.githubusercontent.com/open-feature/spec/main/specification.json')
spec_response = urllib.request.urlopen(
'https://raw.githubusercontent.com/open-feature/spec/main/specification.json'
)
raw = []
for i in spec_response.readlines():
raw.append(i.decode('utf-8'))
Expand All @@ -59,7 +64,8 @@ def get_spec(force_refresh=False, path_prefix="./"):
f.write(data)
return json.loads(data)

def specmap_from_file(actual_spec):

def specmap_from_file(actual_spec: dict[str, Any]) -> dict[str, str]:
spec_map = {}
for entry in actual_spec['rules']:
number = re.search(r'[\d.]+', entry['id']).group()
Expand All @@ -73,12 +79,13 @@ def specmap_from_file(actual_spec):
spec_map[number] = _demarkdown(ch['content'])
return spec_map

def find_covered_specs(config, data):

def find_covered_specs(config: Config, data: str) -> dict[str, dict[str, str]]:
repo_specs = {}
for match in re.findall(config['multiline_regex'], data, re.MULTILINE | re.DOTALL):
match = match.replace('\n', '').replace(config['inline_comment_prefix'], '')
# normalize whitespace
match = re.sub(" {2,}", " ", match.strip())
match = re.sub(' {2,}', ' ', match.strip())
number = re.findall(config['number_subregex'], match)[0]

text_with_concat_chars = re.findall(config['text_subregex'], match, re.MULTILINE | re.DOTALL)
Expand All @@ -94,13 +101,13 @@ def find_covered_specs(config, data):
print(f"Skipping {match} b/c we couldn't parse it")
return repo_specs

def gen_report(from_spec, from_repo):

def gen_report(from_spec: dict[str, str], from_repo: dict[str, dict[str, str]]) -> dict[str, set[str]]:
extra = set()
missing = set()
different_text = set()
good = set()

missing = set(from_spec.keys()) # assume they're all missing
missing = set(from_spec.keys()) # assume they're all missing

for number, text in from_repo.items():
if number in missing:
Expand All @@ -121,7 +128,13 @@ def gen_report(from_spec, from_repo):
}


def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directory=None, json_report=False):
def main(
refresh_spec: bool = False,
diff_output: bool = False,
limit_numbers: str | None = None,
code_directory: str | None = None,
json_report: bool = False,
) -> None:
report = {
'extra': set(),
'missing': set(),
Expand All @@ -134,12 +147,11 @@ def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directo

spec_map = specmap_from_file(actual_spec)


repo_specs = {}
missing = set(spec_map.keys())
bad_num = 0

for root, dirs, files in os.walk(".", topdown=False):
for root, dirs, files in os.walk('.', topdown=False):
for name in files:
F = os.path.join(root, name)
if ('.%s' % config['file_extension']) not in name:
Expand All @@ -153,25 +165,24 @@ def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directo

for number in report['different-text']:
bad_num += 1
print(f"{number} is bad.")
print(f'{number} is bad.')
if diff_output:
print("Official:")
print("\t%s" % spec_map[number])
print("")
print("Ours:")
print("\t%s" % repo_specs[number])
print('Official:')
print('\t%s' % spec_map[number])
print('')
print('Ours:')
print('\t%s' % repo_specs[number])

bad_num += len(report['extra'])
for number in report['extra']:
print(f"{number} is defined in our tests, but couldn't find it in the spec")


missing = report['missing']
bad_num += len(missing)
if len(missing) > 0:
print('In the spec, but not in our tests: ')
for m in sorted(missing):
print(f" {m}: {spec_map[m]}")
print(f' {m}: {spec_map[m]}')

if json_report:
for k in report.keys():
Expand All @@ -190,9 +201,8 @@ def main(refresh_spec=False, diff_output=False, limit_numbers=None, code_directo
parser.add_argument('--refresh-spec', action='store_true', help='Re-downloads the spec')
parser.add_argument('--diff-output', action='store_true', help='print the text differences')
parser.add_argument('--code-directory', action='store', required=True, help='directory to find code in')
parser.add_argument('--json-report', action='store_true', help="Store a json report into ${extension}-report.json")
parser.add_argument('specific_numbers', metavar='num', type=str, nargs='*',
help='limit this to specific numbers')
parser.add_argument('--json-report', action='store_true', help='Store a json report into ${extension}-report.json')
parser.add_argument('specific_numbers', metavar='num', type=str, nargs='*', help='limit this to specific numbers')

args = parser.parse_args()
main(
Expand Down
18 changes: 12 additions & 6 deletions tools/repo_parser/test_spec_finder.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import re
from spec_finder import find_covered_specs, gen_report


def test_simple_singleline():
text = """
// spec:4.3.6:The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.:end
Expand All @@ -13,7 +13,10 @@ def test_simple_singleline():
}
output = find_covered_specs(cfg, text)
assert '4.3.6' in output
assert output['4.3.6']['text'] == "The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value."
assert (
output['4.3.6']['text']
== 'The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.'
)


def test_multiline_comment():
Expand All @@ -31,20 +34,23 @@ def test_multiline_comment():
}
output = find_covered_specs(cfg, text)
assert '4.3.7' in output
assert output['4.3.7']['text'] == """The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value."""
assert (
output['4.3.7']['text']
== """The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value."""
)


def test_report():
spec = {
'1.2.3': "good text",
'1.2.3': 'good text',
'2.3.4': 'different text',
'3.4.5': 'missing'
'3.4.5': 'missing',
}

repo = {
'1.2.3': 'good text',
'2.3.4': 'it is different',
'4.5.6': 'extra'
'4.5.6': 'extra',
}

report = gen_report(spec, repo)
Expand Down

0 comments on commit df1f62e

Please sign in to comment.