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

Arxivce 837 repair test cases #323

Merged
merged 14 commits into from
Oct 23, 2023
Merged
39 changes: 39 additions & 0 deletions .github/workflows/python-app.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# This workflow will install Python dependencies, run tests and lint with a single version of Python
# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python

name: Python application

on:
push:
branches: [ "develop" ]
pull_request:
branches: [ "develop" ]

permissions:
contents: read

jobs:
build:

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v3
- name: Set up Python 3.10
uses: actions/setup-python@v3
with:
python-version: "3.10.9"
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install flake8 pytest poetry
poetry install
#- name: Lint with flake8
# run: |
# # stop the build if there are Python syntax errors or undefined names
# flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uncomment this flake8 test. When I run this test locally it passes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one on line 34. The one on 36 does not yet pass.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I try to run line 34 locally I get TypeError: missing_whitespace() takes 1 positional argument but 2 were given

`(search) erin@Erinarxiv:~/arxiv-search$ flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
multiprocessing.pool.RemoteTraceback:
"""
Traceback (most recent call last):
File "/home/erin/.pyenv/versions/3.10.9/lib/python3.10/multiprocessing/pool.py", line 125, in worker
result = (True, func(*args, **kwds))
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/checker.py", line 82, in _mp_run
).run_checks()
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/checker.py", line 526, in run_checks
self.process_tokens()
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/checker.py", line 511, in process_tokens
self.handle_newline(token_type)
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/checker.py", line 541, in handle_newline
self.run_logical_checks()
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/checker.py", line 448, in run_logical_checks
for offset, text in results:
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/plugins/pycodestyle.py", line 75, in pycodestyle_logical
yield from _missing_whitespace(logical_line, tokens)
TypeError: missing_whitespace() takes 1 positional argument but 2 were given
"""

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/home/erin/.pyenv/versions/search/bin/flake8", line 8, in
sys.exit(main())
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/main/cli.py", line 23, in main
app.run(argv)
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/main/application.py", line 198, in run
self._run(argv)
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/main/application.py", line 187, in _run
self.run_checks()
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/main/application.py", line 103, in run_checks
self.file_checker_manager.run()
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/checker.py", line 235, in run
self.run_parallel()
File "/home/erin/.pyenv/versions/3.10.9/envs/search/lib/python3.10/site-packages/flake8/checker.py", line 204, in run_parallel
self.results = list(pool.imap_unordered(_mp_run, self.filenames))
File "/home/erin/.pyenv/versions/3.10.9/lib/python3.10/multiprocessing/pool.py", line 873, in next
raise value
TypeError: missing_whitespace() takes 1 positional argument but 2 were given`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, leave that along for now then.

# # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
# flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
- name: Test with pytest
run: |
poetry run pytest
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,11 @@ E.g.
WITH_INTEGRATION=1 pipenv run nose2 --with-coverage
```

You can also run to avoid all intentional error messages or kinesis print statements created during the tests
```bash
pytest --disable-warnings search
```

### Static checking
Goal: zero errors/warnings.

Expand Down
12 changes: 4 additions & 8 deletions search/controllers/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,16 @@ class TestHealthCheck(TestCase):
def test_index_is_down(self, mock_index):
"""Test returns 'DOWN' + status 500 when index raises an exception."""
mock_index.search.side_effect = RuntimeError
response, status_code, _ = health_check()
self.assertEqual(response, "DOWN", "Response content should be DOWN")
self.assertEqual(
status_code,
HTTPStatus.INTERNAL_SERVER_ERROR,
"Should return 500 status code.",
)

with self.assertRaises(RuntimeError):
response, status_code, _ = health_check()

@mock.patch("search.controllers.index.SearchSession")
def test_index_returns_no_result(self, mock_index):
"""Test returns 'DOWN' + status 500 when index returns no results."""
mock_index.search.return_value = {"metadata": {}, "results": []}
response, status_code, _ = health_check()
self.assertEqual(response, "DOWN", "Response content should be DOWN")
self.assertEqual(response, "DOWN: document_set lacked results", "Response content should be DOWN: document_set lacked results")
self.assertEqual(
status_code,
HTTPStatus.INTERNAL_SERVER_ERROR,
Expand Down
3 changes: 2 additions & 1 deletion search/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def create_ui_web_app() -> Flask:
for filter_name, template_filter in filters.filters:
app.template_filter(filter_name)(template_filter)

index_startup_check(app)
if app.config["TESTING"]:
index_startup_check(app)
return app


Expand Down
6 changes: 3 additions & 3 deletions search/tests/test_param_persistence.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_request_does_not_include_params(self):
self.assertIn("Set-Cookie", response.headers, "Should set a cookie")
self.assertEqual(
response.headers["Set-Cookie"],
'foo-cookie="{}"; Path=/',
'foo-cookie={}; Path=/',
"Cookie should not contain request params",
)

Expand All @@ -52,7 +52,7 @@ def test_request_includes_cookie(self, mock_simple):
ui.PARAMS_TO_PERSIST = ["foo", "baz"]
ui.PARAMS_COOKIE_NAME = "foo-cookie"
self.client.set_cookie(
"", ui.PARAMS_COOKIE_NAME, json.dumps({"foo": "ack"})
ui.PARAMS_COOKIE_NAME, json.dumps({"foo": "ack"})
)
self.client.get("/")
self.assertEqual(
Expand All @@ -68,7 +68,7 @@ def test_request_includes_cookie_but_also_explicit_val(self, mock_simple):
ui.PARAMS_TO_PERSIST = ["foo", "baz"]
ui.PARAMS_COOKIE_NAME = "foo-cookie"
self.client.set_cookie(
"", ui.PARAMS_COOKIE_NAME, json.dumps({"foo": "ack"})
ui.PARAMS_COOKIE_NAME, json.dumps({"foo": "ack"})
)
self.client.get("/?foo=oof")
self.assertEqual(
Expand Down
6 changes: 4 additions & 2 deletions search/tests/test_searches.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from unittest import TestCase, mock

from search.factory import create_ui_web_app

from search.services.index import QueryError

class TestSearchs(TestCase):
"""Test for the advanced search UI."""
Expand All @@ -13,8 +13,10 @@ def setUp(self):
self.app = create_ui_web_app()
self.client = self.app.test_client()

def test_bad_query(self):
@mock.patch("search.controllers.simple.SearchSession")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

def test_bad_query(self, mock_index):
"""Bad query should result in a 400 not a 500. query from ARXIVNG-2437"""
mock_index.search.side_effect = QueryError
response = self.client.get("/?query=%2F%3F&searchtype=all&source=header")
self.assertEqual(
response.status_code,
Expand Down