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

Update for TestTokenQueries.py and TestWebserver.py #975

Merged
merged 12 commits into from
Sep 10, 2024
Merged
8 changes: 7 additions & 1 deletion emission/core/get_database.py
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
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
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
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
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