Skip to content

Commit

Permalink
[cleanup] Add test script for easier local debugging (#409)
Browse files Browse the repository at this point in the history
Prior to merge, run ./run_test.sh -a to mimic our continuous-integration tests.
Updated README
Also fix bug codeql setup from #408
  • Loading branch information
beets authored Mar 22, 2021
1 parent c3bf348 commit 5b34628
Show file tree
Hide file tree
Showing 11 changed files with 233 additions and 129 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ name: "CodeQL"

on:
push:
branches: [ master, * ]
branches: [ master ]
pull_request:
# The branches below must be a subset of the branches above
branches: [ master ]
Expand Down
24 changes: 17 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,31 +137,41 @@ Consider automating coding to satisfy some of these requirements.
[vim](https://github.com/mindriot101/vim-yapf#why-you-may-not-need-this-plugin).
Specify the Google style using `--style google`.

To run the tools via a command line:
To run the tools via a command line (both installed after setup steps above):

* [pylint](http://pylint.pycqa.org/en/latest/user_guide/run.html).
* After [installing yapf](https://github.com/google/yapf#id2), execute using
* [yapf](https://github.com/google/yapf#id2), execute using
`--style google`, e.g.,

```
# Update (--in-place) all files in the util/ directory and its subdirectories.
yapf --recursive --in-place --style google util/
```shell
# Update (--in-place) all files
./run_tests.sh -f
# Produce differences between the current code and reformatted code. Empty
# output indicates correctly formatted code.
yapf --recursive --diff --style google util/
./run_tests.sh -l
```

To run a unit test, use a command like

```
```shell
python3 -m unittest discover -v -s util/ -p "*_test.py"
```

The `discover` option searches (`-s`) the `util/` directory for files with
filenames ending with `_test.py`. It considers all these files to be unit tests
to be run. Output is verbose (`-v`).

We provide a utility to run all unit tests in a folder easily (e.g. util/):
```shell
./run_tests.sh -p util/
```

Or to run all tests and checks:
```shell
./run_tests.sh -a
```

#### Disabling style checks

Occasionally, one has to disable style checking or formatting for particular
Expand Down
48 changes: 19 additions & 29 deletions cloudbuild.py.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@ steps:

- id: python_install
name: python:3.7
entrypoint: python3
args: ["-m" , "pip" , "install" , "-t" , "." , "-r" , "requirements.txt"]
entrypoint: /bin/sh
waitFor: ['-']
args:
- -c
- |
./run_tests.sh -r
# We can't just test everything under our Cloud Build working directory because
# that's where python installs all of its packages also, and we don't really
Expand All @@ -13,41 +17,27 @@ steps:
# The unittest module only supports one -s path at a time, so we need multiple rules
# to get both places where we submit code.
#
- id: python_test_util
- id: python_test
name: python:3.7
entrypoint: python3
args: ["-m", "unittest", "discover", "-v", "-s", "util/", "-p", "*_test.py"]
# In cloudbuild, everything happens under /workspace path
env: ["PYTHONPATH=/workspace:/workspace/scripts"]
waitFor:
- python_install

- id: python_test_scripts
name: python:3.7
entrypoint: python3
args: ["-m", "unittest", "discover", "-v", "-s", "scripts/", "-p", "*_test.py"]
# In cloudbuild, everything happens under /workspace path
env: ["PYTHONPATH=/workspace:/workspace/scripts"]
waitFor:
- python_install

- id: python_test_import_automation
name: python:3.7
entrypoint: python3
args: ["-m", "unittest", "discover", "-v", "-s", "import-automation/", "-p", "*_test.py"]
# In cloudbuild, everything happens under /workspace path
env: ["PYTHONPATH=/workspace:/workspace/scripts"]
entrypoint: /bin/sh
waitFor:
- python_install
args:
- -c
- |
./run_tests.sh -p util/
./run_tests.sh -p import-automation/
./run_tests.sh -p scripts/
- id: python_format_check
name: python:3.7
entrypoint: python3
args: ["-m", "yapf", "--recursive", "--diff", "--style", "google", "util/", "scripts/", "tools/", "docs/"]
# In cloudbuild, everything happens under /workspace path
env: ["PYTHONPATH=/workspace:/workspace/scripts"]
entrypoint: /bin/sh
waitFor:
- python_install
args:
- -c
- |
./run_tests.sh -l
# TODO(rsned): Uncomment once the existing source files are updated to pass lint.
#- id: python_lint
Expand Down
7 changes: 5 additions & 2 deletions import-automation/executor/app/executor/update_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class UpdateScheduler:
Returns:
Created job as a dict.
"""

def __init__(self,
client: scheduler.CloudSchedulerClient,
github: github_api.GitHubRepoAPI,
Expand Down Expand Up @@ -156,8 +157,10 @@ def _schedule_on_commit_helper(

if self.dashboard:
self.dashboard.update_run(
{'status': 'succeeded', 'time_completed': utils.utctime()},
run_id)
{
'status': 'succeeded',
'time_completed': utils.utctime()
}, run_id)
return import_executor.ExecutionResult('succeeded', scheduled,
'No issues')

Expand Down
8 changes: 4 additions & 4 deletions import-automation/executor/app/service/dashboard_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ def init_run(self, system_run: Dict) -> Dict:
requests.HTTPError: The importer returns a status code that is
larger than or equal to 400.
"""
logging.info('DashboardAPI.init_run: Posting %s to %s',
system_run, _DASHBOARD_RUN_LIST)
logging.info('DashboardAPI.init_run: Posting %s to %s', system_run,
_DASHBOARD_RUN_LIST)
response = self.iap.post(_DASHBOARD_RUN_LIST, json=system_run)
response.raise_for_status()
logging.info('DashboardAPI.init_run: Received %s from %s',
Expand Down Expand Up @@ -189,8 +189,8 @@ def update_run(self, system_run: Dict, run_id: str) -> Dict:
larger than or equal to 400.
"""
query = _DASHBOARD_RUN_BY_ID.format_map({'run_id': run_id})
logging.info('DashboardAPI.update_run: Patching %s to %s',
system_run, query)
logging.info('DashboardAPI.update_run: Patching %s to %s', system_run,
query)
response = self.iap.patch(query, json=system_run)
response.raise_for_status()
logging.info('DashboardAPI.update_run: Received %s from %s',
Expand Down
12 changes: 5 additions & 7 deletions import-automation/executor/app/service/email_notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ def __init__(self,
self.account = account
self.password = password
self.host = host
logging.info('EmailNotifier.__init__: '
'Initialized with account %s and host %s',
account, host)
logging.info(
'EmailNotifier.__init__: '
'Initialized with account %s and host %s', account, host)

def send(self, subject: str, body: str,
receiver_addresses: List[str]) -> None:
Expand All @@ -64,10 +64,8 @@ def send(self, subject: str, body: str,
f'Subject: {subject}\n'
'\n'
f'{body}\n')
logging.info('EmailNotifier.send: Sending %s to %s',
email, receivers)
logging.info('EmailNotifier.send: Sending %s to %s', email, receivers)
with smtplib.SMTP_SSL(self.host) as server:
server.login(self.account, self.password)
server.sendmail(self.account, receiver_addresses, email)
logging.info('EmailNotifier.send: Sent %s to %s',
email, receivers)
logging.info('EmailNotifier.send: Sent %s to %s', email, receivers)
33 changes: 17 additions & 16 deletions import-automation/executor/app/service/file_uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ def __init__(self,
_strings_not_empty(project_id, bucket_name)
self.bucket = storage.Client(project=project_id).bucket(bucket_name)
self.path_prefix = path_prefix
logging.info('GCSFileUploader.__init__: '
'Initialized with bucket %s at prefix %s on project %s',
bucket_name, path_prefix, project_id)
logging.info(
'GCSFileUploader.__init__: '
'Initialized with bucket %s at prefix %s on project %s',
bucket_name, path_prefix, project_id)

def upload_file(self, src: str, dest: str) -> None:
"""Uploads a file to the bucket.
Expand All @@ -81,12 +82,12 @@ def upload_file(self, src: str, dest: str) -> None:
"""
_strings_not_empty(src, dest)
dest = self._fix_path(dest)
logging.info('GCSFileUploader.upload_file: Uploading %s to %s',
src, dest)
logging.info('GCSFileUploader.upload_file: Uploading %s to %s', src,
dest)
blob = self.bucket.blob(dest)
blob.upload_from_filename(src)
logging.info('GCSFileUploader.upload_file: Uploaded %s to %s',
src, dest)
logging.info('GCSFileUploader.upload_file: Uploaded %s to %s', src,
dest)

def upload_string(self, string: str, dest: str) -> None:
"""Uploads a string to a file in the bucket, overwriting it.
Expand All @@ -105,8 +106,8 @@ def upload_string(self, string: str, dest: str) -> None:
string, dest)
blob = self.bucket.blob(dest)
blob.upload_from_string(string)
logging.info('GCSFileUploader.upload_string: Uploaded %s to %s',
string, dest)
logging.info('GCSFileUploader.upload_string: Uploaded %s to %s', string,
dest)

def _fix_path(self, path):
"""Returns {self.path_prefix}/{path}."""
Expand All @@ -124,9 +125,9 @@ class LocalFileUploader(FileUploader):

def __init__(self, output_dir: str = ''):
self.output_dir = os.path.abspath(output_dir)
logging.info('LocalFileUploader.__init__: '
'Initialized with output directory %s',
output_dir)
logging.info(
'LocalFileUploader.__init__: '
'Initialized with output directory %s', output_dir)

def upload_file(self, src: str, dest: str) -> None:
"""Copies the file at src to a file at <output_dir>/<dest>.
Expand All @@ -137,12 +138,12 @@ def upload_file(self, src: str, dest: str) -> None:
"""
_strings_not_empty(src, dest)
dest = os.path.join(self.output_dir, dest)
logging.info('LocalFileUploader.upload_file: Uploading %s to %s',
src, dest)
logging.info('LocalFileUploader.upload_file: Uploading %s to %s', src,
dest)
os.makedirs(os.path.dirname(dest), exist_ok=True)
shutil.copyfile(src, dest)
logging.info('LocalFileUploader.upload_file: Uploaded %s to %s',
src, dest)
logging.info('LocalFileUploader.upload_file: Uploaded %s to %s', src,
dest)

def upload_string(self, string: str, dest: str) -> None:
"""Writes a string into a file at <output_dir>/<dest>, overwriting any
Expand Down
Loading

0 comments on commit 5b34628

Please sign in to comment.