Skip to content

Commit

Permalink
✅ Unify the expected messages through redaction
Browse files Browse the repository at this point in the history
Principled fix for the issue outlined in
#975

The fundamental issue is that we check the output to validate that the
script is behaving as expected. However, the script output can vary in
different environments.

We had originally handled this by having multiple expected output strings. But
as we expand the scope of what we print out, the set of expected output strings expanded. Some of the strings also included paths, so had the username embedded, so would be hard to enumerate in the test.

Those logs are also not critical to this test, which is focused on the response
to various arguments passed in to the script.

So we fix this by simply removing the parts that vary across environments, replacing them by `REDACTED`.

As a bonus, this also allows us to collapse the previous two strings into one,
and supports all three testing environments: (i) locally on laptop, (ii)
locally in docker, and (iii) docker CI/CD (although (ii) is not really
supported since it is not tested extensively).

Testing done:

```
$ ./e-mission-py.bash emission/tests/storageTests/TestTokenQueries.py
----------------------------------------------------------------------
Ran 21 tests in 22.862s

OK
```
  • Loading branch information
shankari committed Sep 1, 2024
1 parent 9bba6c8 commit b2aefce
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 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 @@ -172,21 +173,21 @@ def test_run_script_show(self):
# 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)
self.assertIn(sp.stdout,
[b'Config file not found, returning a copy of the environment variables instead...\nRetrieved config: {\'DB_HOST\': None, \'DB_RESULT_LIMIT\': None}\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: {\'DB_HOST\': \'db\', \'DB_RESULT_LIMIT\': None}\nURL not formatted, defaulting to "Stage_database"\nConnecting to database URL db\nx\ny\nz\n'
])

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 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)
self.assertIn(sp.stdout,
[b'Config file not found, returning a copy of the environment variables instead...\nRetrieved config: {\'DB_HOST\': None, \'DB_RESULT_LIMIT\': None}\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: {\'DB_HOST\': \'db\', \'DB_RESULT_LIMIT\': None}\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'
])
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

0 comments on commit b2aefce

Please sign in to comment.