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.

Sign up for GitHub

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

Jump to bottom

Update for TestTokenQueries.py and TestWebserver.py #975

Merged
merged 12 commits into from
Sep 10, 2024
Merged

Update for TestTokenQueries.py and TestWebserver.py #975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
d0bc264
Added dynamic conda environment name to assertion log messages
Aug 21, 2024
1dc4275
Changed TestWebserver.py to work with both config files and ENV varia…
Aug 22, 2024
7ac46aa
Logging CONDA_DEFAULT_ENV to investigate failed tests
Aug 22, 2024
32681cf
Corrected 1st log message for local emission environment
Aug 22, 2024
769346c
Correcting import for restoreOriginalEnvVars
Aug 23, 2024
39885a3
Added fourth message for running tests in local docker container
Aug 23, 2024
9ab6358
Revert "Changed TestWebserver.py to work with both config files and E…
Aug 23, 2024
1a13a49
Added WEBSERVER_NOT_FOUND_REDIRECT to config map
Aug 23, 2024
0efbdc8
Simplified tests with two log messages for four test environments
Aug 26, 2024
9bba6c8
Logging retrieved db config values before using default values
Aug 29, 2024
b2aefce
✅ Unify the expected messages through redaction
shankari Sep 1, 2024
6368598
Added missing import for 'os' module
Sep 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion emission/core/get_database.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@
config = ecbc.get_config('conf/storage/db.conf',
{"DB_HOST": "timeseries.url", "DB_RESULT_LIMIT": "timeseries.result_limit"})

print("Retrieved config %s" % config)
db_config = {}
for key in ["DB_HOST", "DB_RESULT_LIMIT"]:
if key in config:
db_config[key] = config.get(key)
else:
db_config[key] = None
print("Retrieved config: %s" % db_config)
Comment on lines +18 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't like the approach of changing code to make the tests pass, particularly when this is not even critical to the functionality exercised by the failing test.

The test that is failing really needs to check whether the script is working properly or not.

To rephrase, the previous logs actually have other benefits in terms of logging - for example, they can tell us whether the variables came from the environment (there are other variables) or from the config file (there are not).

This may even be meaningful if this was affecting multiple tests.

But this is really only affecting one test, which doesn't even care about the values of the variables, and only cares about the operation of the script with various input options. So changing the code doesn't appear to be the correct approach to fix this.

url = config.get("DB_HOST", "localhost")
result_limit = config.get("DB_RESULT_LIMIT", 250000)

Expand Down
4 changes: 3 additions & 1 deletion emission/integrationTests/storageTests/TestMongodbAuth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
import importlib
import os

import emission.tests.common as etc

class TestMongodbAuth(unittest.TestCase):
def setUp(self):
self.admin_default = pymongo.MongoClient('localhost', username="admin",
Expand All @@ -53,7 +55,7 @@ def setUp(self):
def tearDown(self):
self.admin_auth.command({"dropAllUsersFromDatabase": 1})
logging.debug("Deleting test db environment variables")
ecc.restoreOriginalEnvVars(self.originalDBEnvVars, self.modifiedEnvVars)
etc.restoreOriginalEnvVars(self.originalDBEnvVars, self.modifiedEnvVars)
logging.debug("Finished restoring original db environment variables")
logging.debug("Restored original values are = %s" % self.originalDBEnvVars)
try:
Expand Down
3 changes: 2 additions & 1 deletion emission/net/api/cfc_webapp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@

config = ecbc.get_config('conf/net/api/webserver.conf',
{"WEBSERVER_PORT": "server.port", "WEBSERVER_TIMEOUT": "server.timeout",
"WEBSERVER_AUTH": "server.auth", "WEBSERVER_AGGREGATE_CALL_AUTH": "server.aggregate_call_auth"})
"WEBSERVER_AUTH": "server.auth", "WEBSERVER_AGGREGATE_CALL_AUTH": "server.aggregate_call_auth",
"WEBSERVER_NOT_FOUND_REDIRECT": "paths.404_redirect"})
server_port = config.get("WEBSERVER_PORT", 8080)
socket_timeout = config.get("WEBSERVER_TIMEOUT", 3600)
auth_method = config.get("WEBSERVER_AUTH", "skip")
Expand Down
1 change: 1 addition & 0 deletions emission/net/ext_service/push/notify_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import json
import logging
import importlib
import os

import emission.core.backwards_compat_config as ecbc

Expand Down
33 changes: 17 additions & 16 deletions emission/tests/storageTests/TestTokenQueries.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import uuid
import json
import os
import re

#changed all script runs from os() to subprocess.run() for consistency
#TODO clean up commented out os() lines
Expand Down Expand Up @@ -168,25 +169,25 @@ def test_run_script_show(self):
esdt.insert({'token':'y'})
esdt.insert({'token':'z'})
sp = subprocess.run(["python3", "bin/auth/insert_tokens.py", "--show"], capture_output=True)
# The first message is displayed when we run tests locally
# The second is displayed when we run in the CI/CD, but with the local install
# The third is displayed when we run in the docker CI since the `DB_HOST` is set to `db`
self.assertIn(sp.stdout,
[b'Retrieved config {\'DB_HOST\': \'localhost\', \'DB_RESULT_LIMIT\': 250000}\nURL not formatted, defaulting to "Stage_database"\nConnecting to database URL localhost\nx\ny\nz\n',
b'Config file not found, returning a copy of the environment variables instead...\nRetrieved config {\'PYTHONPATH\': \'.\', \'PATH\': \'/home/runner/miniconda-23.5.2/envs/emissiontest/bin:/home/runner/miniconda-23.5.2/condabin:/snap/bin:/home/runner/.local/bin:/opt/pipx_bin:/home/runner/.cargo/bin:/home/runner/.config/composer/vendor/bin:/usr/local/.ghcup/bin:/home/runner/.dotnet/tools:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/runner/.dotnet/tools\', \'LC_CTYPE\': \'C.UTF-8\'}\nURL not formatted, defaulting to "Stage_database"\nConnecting to database URL localhost\nx\ny\nz\n',
b'Config file not found, returning a copy of the environment variables instead...\nRetrieved config {\'PYTHONPATH\': \'.\', \'DB_HOST\': \'db\', \'PATH\': \'/root/miniconda-23.5.2/envs/emissiontest/bin:/root/miniconda-23.5.2/condabin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\', \'LC_CTYPE\': \'C.UTF-8\'}\nURL not formatted, defaulting to "Stage_database"\nConnecting to database URL db\nx\ny\nz\n'
])
# The first message is displayed when we run tests locally or run in the CI/CD, but with the local install
# The second is displayed when we run tests (the `DB_HOST` is set to `db` by default):
# a) in the docker CI or,
# b) locally in a docker container (ad-hoc testing environment; do not expect this to be used)

stripped_out_stdout = re.sub(rb'Retrieved config.*\nURL not formatted.*\nConnecting to database.*', b'REDACTED', sp.stdout)
self.assertEqual(stripped_out_stdout,
b'Config file not found, returning a copy of the environment variables instead...\nREDACTED\nx\ny\nz\n')

def test_run_script_empty(self):
sp = subprocess.run(["python3", "bin/auth/insert_tokens.py"], capture_output=True)
# The first message is displayed when we run tests locally
# The second is displayed when we run in the CI/CD, but with the local install
# The third is displayed when we run in the docker CI since the `DB_HOST` is set to `db`
self.assertIn(sp.stdout,
[b'Retrieved config {\'DB_HOST\': \'localhost\', \'DB_RESULT_LIMIT\': 250000}\nURL not formatted, defaulting to "Stage_database"\nConnecting to database URL localhost\nPlease provide the script with an argument. Use the "--help" option for more details\n',
b'Config file not found, returning a copy of the environment variables instead...\nRetrieved config {\'PYTHONPATH\': \'.\', \'PATH\': \'/home/runner/miniconda-23.5.2/envs/emissiontest/bin:/home/runner/miniconda-23.5.2/condabin:/snap/bin:/home/runner/.local/bin:/opt/pipx_bin:/home/runner/.cargo/bin:/home/runner/.config/composer/vendor/bin:/usr/local/.ghcup/bin:/home/runner/.dotnet/tools:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/runner/.dotnet/tools\', \'LC_CTYPE\': \'C.UTF-8\'}\nURL not formatted, defaulting to "Stage_database"\nConnecting to database URL localhost\nPlease provide the script with an argument. Use the "--help" option for more details\n',
b'Config file not found, returning a copy of the environment variables instead...\nRetrieved config {\'PYTHONPATH\': \'.\', \'DB_HOST\': \'db\', \'PATH\': \'/root/miniconda-23.5.2/envs/emissiontest/bin:/root/miniconda-23.5.2/condabin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\', \'LC_CTYPE\': \'C.UTF-8\'}\nURL not formatted, defaulting to "Stage_database"\nConnecting to database URL db\nPlease provide the script with an argument. Use the "--help" option for more details\n'
])
# The first message is displayed when we run tests locally or run in the CI/CD, but with the local install
# The second is displayed when we run tests (the `DB_HOST` is set to `db` by default):
# a) in the docker CI or,
# b) locally in a docker container (ad-hoc testing environment; do not expect this to be used)
stripped_out_stdout = re.sub(rb'Retrieved config.*\nURL not formatted.*\nConnecting to database.*', b'REDACTED', sp.stdout)
self.assertEqual(stripped_out_stdout,
b'Config file not found, returning a copy of the environment variables instead...\nREDACTED\nPlease provide the script with an argument. Use the "--help" option for more details\n'
)

#test that no two options can be used together
def test_run_script_mutex(self):
Expand Down
Loading