Skip to content

Commit

Permalink
chore: add pylint validation (hedyorg#440)
Browse files Browse the repository at this point in the history
Add `pylint` into the standard set of tests. `pylint` is a style
checker for Python. There are currently many style issues with the code
base, so the suite of generic style checking has been disabled. Instead,
we just check for obvious errors (misspellings of variables, etc) and
throw an error if we find any.

`pylint` found 3 in the current code base:

- Two functions in a testing class that had the same name (so the
  2nd function definition would overwrite the first). Removed the first
  test, since it was trying forms that were illegal at level 2 anyway, so
  could never work.
- Illegal access of `.line`, `.column`, `.allowed` on an instance of
  something that's declared as `Exception`. Some investigation teaches
  that the only Lark exception that has all 3 of these attributes is
  `UnexpectedCharacters`, so changed to catching only that exception.
- Some issues where we were assigning to fields of `g` (`g.prefix = ...`),
  which is allowed by pylint couldn't determine that from available declarations.
  Upgrading Flask from `1.1.1` -> `1.1.4` fixed that.

Added scripts to do all of the validations we check:

```
build-tools/validate-python
build-tools/validate-tests
build-tools/validate-yaml
```

And a single script:

```
build-tools/validate
```

To run all validations.

All validations are now run (individually) in the PR validation workflow
script as well.

Changing the workflow version of python to 3.9, since that's the version we're
using on Heroku.
  • Loading branch information
rix0rrr authored May 23, 2021
1 parent cc06400 commit 355eed1
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 13 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/unittests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ jobs:

steps:
- uses: actions/checkout@v2
- name: Set up Python 3.8
- name: Set up Python 3.9
uses: actions/setup-python@v1
with:
python-version: 3.8
python-version: 3.9
- name: Install dependencies
run: |
python3 -m pip install --upgrade pip
pip3 install -r requirements.txt
- name: Test Python for errors
run: build-tools/validate-python
- name: Test with unittest
run: |
python -m unittest discover -s tests
run: build-tools/validate-tests
- name: Validate YAML
run: build-tools/validate-yaml

11 changes: 11 additions & 0 deletions build-tools/validate
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash
# Run all validation scripts
set -eu
scriptdir=$(cd $(dirname $0) && pwd)


$scriptdir/validate-python
$scriptdir/validate-tests
$scriptdir/validate-yaml

echo "Everything is great! 🍰"
11 changes: 11 additions & 0 deletions build-tools/validate-python
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/bin/bash
# Run pylint against all Python code
set -eu
scriptdir=$(cd $(dirname $0) && pwd)
cd $scriptdir/..

# There are MANY pep8 style violations in the code base currently.
# Let's just check for definite errors.
pylint -j 0 \
--errors-only \
*.py tests
8 changes: 8 additions & 0 deletions build-tools/validate-tests
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/bin/bash
# Helper script to run unit tests
set -eu
scriptdir=$(cd $(dirname $0) && pwd)
cd $scriptdir/..

# This is expected to run from the repo root
python -m unittest discover -s tests
6 changes: 3 additions & 3 deletions hedy.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from lark import Lark
from lark.exceptions import VisitError, LarkError, UnexpectedEOF
from lark import Tree, Transformer, Visitor
from lark.exceptions import LarkError, UnexpectedEOF, UnexpectedCharacters
from lark import Tree, Transformer
from os import path
import sys
import utils
Expand Down Expand Up @@ -1053,7 +1053,7 @@ def transpile_inner(input_string, level):
abstract_syntaxtree = ExtractAST().transform(program_root)
lookup_table = AllAssignmentCommands().transform(abstract_syntaxtree)

except Exception as e:
except UnexpectedCharacters as e:
try:
location = e.line, e.column
characters_expected = str(e.allowed)
Expand Down
3 changes: 2 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Flask==1.1.1
Flask==1.1.4
lark==0.10.1
lark-parser==0.7.8
gunicorn==19.9.0
Expand All @@ -9,3 +9,4 @@ Flask-Commonmark==0.8
bcrypt==3.2.0
boto3==1.16.50
ruamel.yaml==0.17.4
pylint==2.8.2
4 changes: 0 additions & 4 deletions tests/tests_level_02.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,6 @@ def test_transpile_other(self):
result = hedy.transpile("abc felienne 123", 2)
self.assertEqual('Invalid', str(context.exception))

def test_transpile_ask_Spanish(self):
result = hedy.transpile("ask ask Cuál es tu color favorito?", 2)
self.assertEqual("answer = input('ask Cuál es tu color favorito?')", result)

def test_transpile_echo_at_level_2(self):
with self.assertRaises(Exception) as context:
result = hedy.transpile("echo Jouw lievelingskleur is dus...", 2)
Expand Down

0 comments on commit 355eed1

Please sign in to comment.