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

Conversation

kyokukou
Copy link
Contributor

repaired 5 failing test cases

healthcheck documentation says it should be creating an error
removed extra(?) empty string parameter
which was causing no cookies to be set
they caused assertion to fail
was previously trying to query actual ES service
helps avoid some of the mess that prints during nose tests
@@ -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.

👍

@kyokukou kyokukou requested a review from bdc34 October 19, 2023 20:41
@kyokukou
Copy link
Contributor Author

also added in feature for autorunning tests

#- 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.

@@ -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"] =="ON":
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to skip the index_startup_check() when the testing is happening and to run it when we are not doing testing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this isn't "ON" or "OFF" It will be of type bool

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think a boolean would be a lot more practical than a string, but I already found this in the config file. (which does still mean I should be changing the string I'm looking for"
`ON = "yes"
OFF = "no"

DEBUG = os.environ.get("DEBUG") == ON
"""enable/disable debug mode"""

TESTING = os.environ.get("TESTING") == ON
"""enable/disable testing mode"""
`

Copy link
Contributor

Choose a reason for hiding this comment

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

Notice that it is using == so the result is a bool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahhhhhh. You're right, ty

Copy link
Contributor

@bdc34 bdc34 left a comment

Choose a reason for hiding this comment

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

Looks good and I'll be glad for the tests to work.

@kyokukou kyokukou requested a review from bdc34 October 20, 2023 14:27
@kyokukou kyokukou merged commit eb93adf into develop Oct 23, 2023
1 check passed
@kyokukou kyokukou deleted the ARXIVCE-837-repair-test-cases branch October 23, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants