Skip to content

Commit

Permalink
chore(tests): Make them work
Browse files Browse the repository at this point in the history
There were several issues with these tests preventing them from working.

* The parallelism caused port collision despite using a random port
* The `run_before` section in tox.ini caused several issues in parallel
  as it was killing the testing harness for other tests while running
* There is a bug in flask-restful which prevents it from correctly
  parsing args if you do not specify location and are running inside a
  virtual environment. flask-restful/flask-restful#936
* Moto changed it's testing signature requiring arity of two.
* The psycopg2 manually installed version doesn't seem to work correctly
  in the tests and there are comments saying as much
  • Loading branch information
bkochendorfer committed Jun 16, 2022
1 parent d0d3ef6 commit 4aa97e3
Show file tree
Hide file tree
Showing 18 changed files with 52 additions and 36 deletions.
4 changes: 2 additions & 2 deletions Makefile
Expand Up @@ -77,13 +77,13 @@ push-ci-container:
.PHONY: test
test:
docker-compose build
docker-compose run --rm tester bash -c '/root/utils/fake-creds.sh && source /root/.bashrc && make -C python-modules -j$(nproc) test-tox'
docker-compose run --rm tester bash -c '/root/utils/fake-creds.sh && source /root/.bashrc && make -C python-modules test-tox'

.PHONE: test-module
test-module:
docker-compose build
@echo "Testing single module: $(MODULE)"
docker-compose run --rm tester bash -c '/root/utils/fake-creds.sh && source /root/.bashrc && make -C python-modules/$(MODULE) -j$(nproc) test-tox'
docker-compose run --rm tester bash -c '/root/utils/fake-creds.sh && source /root/.bashrc && make -C python-modules/$(MODULE) test-tox'

.PHONY: developer-shell
developer-shell:
Expand Down
2 changes: 1 addition & 1 deletion ci/Dockerfile
Expand Up @@ -4,7 +4,7 @@ LABEL maintainer="akrug@mozilla.com"
# skip global package update - whatever archlinux/latest has is enough
#RUN pacman -Syyu --noconfirm
# install arch base-devel, dependencies, and refresh the package list from upstream
RUN pacman --noconfirm -S -y --needed base-devel iputils net-tools grep nodejs npm docker make pacman-contrib jq python-pip zip gcc python2 wget unzip tar gawk jdk-openjdk
RUN pacman --noconfirm -S -y --needed base-devel iputils net-tools grep nodejs npm docker make pacman-contrib jq python-pip zip gcc python2 wget unzip tar gawk jdk-openjdk postgresql-libs python-psycopg2
## Use this if you need to force-downgrade system's python
## You'll want to compile the version you want first and upload it to S3
# RUN wget https://s3-us-west-2.amazonaws.com/public.iam.mozilla.com/python37-3.7.5-2-x86_64.pkg.tar.xz
Expand Down
4 changes: 2 additions & 2 deletions python-modules/cis_aws/tests/test_connect.py
Expand Up @@ -18,8 +18,8 @@

class TestConnect(object):
def setup(self):
self.dynalite_port = str(random.randint(32000, 34000))
self.kinesalite_port = str(random.randint(32000, 34000))
self.dynalite_port = str(random.randint(32000, 32100))
self.kinesalite_port = str(random.randint(32100, 32200))
os.environ["CIS_DYNALITE_PORT"] = self.dynalite_port
os.environ["CIS_KINESALITE_PORT"] = self.kinesalite_port
self.dynalite_host = "localhost"
Expand Down
4 changes: 2 additions & 2 deletions python-modules/cis_change_service/tests/test_api.py
Expand Up @@ -72,7 +72,7 @@ def ensure_appropriate_publishers_and_sign(fake_profile, publisher_rules, condit

class TestAPI(object):
def setup(self):
self.dynalite_port = str(random.randint(32000, 34000))
self.dynalite_port = str(random.randint(32200, 32300))
os.environ["CIS_CONFIG_INI"] = "tests/mozilla-cis.ini"
os.environ["AWS_XRAY_SDK_ENABLED"] = "false"
os.environ["CIS_ENVIRONMENT"] = "local"
Expand All @@ -87,7 +87,7 @@ def setup(self):
self.mock_salt = self.patcher_salt.start()

config = get_config()
os.environ["CIS_DYNALITE_PORT"] = str(random.randint(32000, 34000))
os.environ["CIS_DYNALITE_PORT"] = str(random.randint(32300, 32400))
self.dynalite_port = config("dynalite_port", namespace="cis")
self.dynaliteprocess = subprocess.Popen(
[
Expand Down
2 changes: 1 addition & 1 deletion python-modules/cis_change_service/tests/test_profile.py
Expand Up @@ -28,7 +28,7 @@ def setup(self):
os.environ["AWS_XRAY_SDK_ENABLED"] = "false"
os.environ["CIS_ENVIRONMENT"] = "local"
os.environ["CIS_CONFIG_INI"] = "tests/mozilla-cis.ini"
self.dynalite_port = str(random.randint(32000, 34000))
self.dynalite_port = str(random.randint(32400, 32500))
os.environ["CIS_DYNALITE_PORT"] = self.dynalite_port
self.dynaliteprocess = subprocess.Popen(
[
Expand Down
2 changes: 2 additions & 0 deletions python-modules/cis_identity_vault/setup.py
Expand Up @@ -32,6 +32,8 @@
"moto>=1.3.7",
"flake8",
"pytest-benchmark",
"psycopg2",
"psycopg2-binary",
]

extras = {"test": test_requirements}
Expand Down
2 changes: 1 addition & 1 deletion python-modules/cis_identity_vault/tests/test_rds.py
Expand Up @@ -5,7 +5,7 @@

@mock_ssm
class TestRDS(object):
def setup(self):
def setup(self, *args):
os.environ["CIS_ENVIRONMENT"] = "testing"
os.environ["CIS_REGION_NAME"] = "us-east-1"
os.environ["DEFAULT_AWS_REGION"] = "us-east-1"
Expand Down
2 changes: 1 addition & 1 deletion python-modules/cis_identity_vault/tests/test_user_model.py
Expand Up @@ -17,7 +17,7 @@

@mock_dynamodb2
class TestUsersDynalite(object):
def setup(self):
def setup(self, *args):
os.environ["CIS_ENVIRONMENT"] = "purple"
os.environ["CIS_REGION_NAME"] = "us-east-1"
os.environ["DEFAULT_AWS_REGION"] = "us-east-1"
Expand Down
2 changes: 1 addition & 1 deletion python-modules/cis_identity_vault/tests/test_vault.py
Expand Up @@ -22,7 +22,7 @@ def test_crud_it_should_succeed(self):

class TestVaultDynalite(object):
def setup_class(self):
self.dynalite_port = str(random.randint(32000, 34000))
self.dynalite_port = str(random.randint(32500, 32600))
os.environ["CIS_DYNALITE_PORT"] = self.dynalite_port
self.dynaliteprocess = subprocess.Popen(
[
Expand Down
6 changes: 4 additions & 2 deletions python-modules/cis_identity_vault/tox.ini
Expand Up @@ -15,5 +15,7 @@ deps=
tox-run-before
../cis_profile
../cis_crypto
/psycopg2-2.8.3
commands=pytest tests/ --cov=cis_identity_vault {posargs}

commands =
pip install psycopg2 psycopg2-binary
pytest tests/ --cov=cis_identity_vault {posargs}
13 changes: 12 additions & 1 deletion python-modules/cis_postgresql/setup.py
Expand Up @@ -10,7 +10,18 @@

setup_requirements = ["pytest-runner", "setuptools>=40.5.0"]

test_requirements = ["pytest", "pytest-watch", "pytest-cov", "patch", "mock", "flake8", "moto", "docker"]
test_requirements = [
"pytest",
"pytest-watch",
"pytest-cov",
"patch",
"mock",
"flake8",
"moto",
"docker",
"psycopg2",
"psycopg2-binary",
]

extras = {"test": test_requirements}

Expand Down
2 changes: 1 addition & 1 deletion python-modules/cis_postgresql/tests/test_handler.py
Expand Up @@ -65,7 +65,7 @@ def events_and_users(self):

@mock_dynamodb2
class TestEventHandler(object):
def setup(self):
def setup(self, *args):
fh = open("tests/fixture/dynamodb-event.json")
# Event data structure to load in order to mock a profile update
self.event_json = fh.read()
Expand Down
5 changes: 3 additions & 2 deletions python-modules/cis_postgresql/tox.ini
Expand Up @@ -17,6 +17,7 @@ deps=
../cis_identity_vault
../cis_profile
../cis_crypto
/psycopg2-2.8.3

commands=pytest tests/ --cov=cis_postgresql {posargs}
commands =
pip install psycopg2 psycopg2-binary
pytest tests/ --cov=cis_postgresql {posargs}
2 changes: 1 addition & 1 deletion python-modules/cis_processor/tests/test_operation.py
Expand Up @@ -57,7 +57,7 @@ def kinesis_event_generate(user_profile):

@mock_dynamodb2
class TestOperation(object):
def setup(self):
def setup(self, *args):
os.environ["CIS_CONFIG_INI"] = "tests/fixture/mozilla-cis.ini"
self.config = get_config()
from cis_profile import WellKnown
Expand Down
Expand Up @@ -36,7 +36,7 @@ def kinesis_event_generate(user_profile):

@mock_dynamodb2
class TestProfileDelegate(object):
def setup(self):
def setup(self, *args):
os.environ["CIS_CONFIG_INI"] = "tests/fixture/mozilla-cis.ini"
self.dynamodb_client = boto3.client(
"dynamodb", region_name="us-west-2", aws_access_key_id="ak", aws_secret_access_key="sk"
Expand Down
Expand Up @@ -96,21 +96,21 @@ class v2UsersByAttrContains(Resource):
decorators = [requires_auth]

def get(self):
parser = reqparse.RequestParser()
parser = reqparse.RequestParser(bundle_errors=True)
parser.add_argument("Authorization", location="headers")
parser.add_argument("filterDisplay", type=str)
parser.add_argument("filterDisplay", type=str, location="args")

for attr in allowed_advanced_queries:
# Ensure that only our allowed attributes are parsed.
parser.add_argument(attr, type=allowed_advanced_queries[attr])
parser.add_argument(attr, type=allowed_advanced_queries[attr], location="args")

for attr in allowed_advanced_queries:
# Ensure that only our allowed attributes as inverse ops are parsed.
parser.add_argument(f"not_{attr}", type=allowed_advanced_queries[attr])
parser.add_argument(f"not_{attr}", type=allowed_advanced_queries[attr], location="args")

parser.add_argument("active", type=bool)
parser.add_argument("fullProfiles", type=bool)
parser.add_argument("nextPage", type=str)
parser.add_argument("active", type=bool, location="args")
parser.add_argument("fullProfiles", type=bool, location="args")
parser.add_argument("nextPage", type=str, location="args")

# determine which arg was passed in from the whitelist and then set it up
reserved_keys = ["active", "nextPage", "fullProfiles", "Authorization", "filterDisplay"]
Expand Down
Expand Up @@ -156,9 +156,9 @@ class v2UsersByAny(Resource):

def get(self):
parser = reqparse.RequestParser()
parser.add_argument("connectionMethod", type=str)
parser.add_argument("active", type=str)
parser.add_argument("nextPage", type=str)
parser.add_argument("connectionMethod", type=str, location="args")
parser.add_argument("active", type=str, location="args")
parser.add_argument("nextPage", type=str, location="args")

args = parser.parse_args()

Expand Down Expand Up @@ -213,8 +213,8 @@ def getUser(id, find_by):
id = urllib.parse.unquote(id)
parser = reqparse.RequestParser()
parser.add_argument("Authorization", location="headers")
parser.add_argument("filterDisplay", type=str)
parser.add_argument("active", type=str)
parser.add_argument("filterDisplay", type=str, location="args")
parser.add_argument("active", type=str, location="args")
args = parser.parse_args()
scopes = get_scopes(args.get("Authorization"))
filter_display = args.get("filterDisplay", None)
Expand Down Expand Up @@ -276,10 +276,10 @@ def get(self):
"""Return a single user with id `user_id`."""
parser = reqparse.RequestParser()
parser.add_argument("Authorization", location="headers")
parser.add_argument("nextPage", type=str)
parser.add_argument("primaryEmail", type=str)
parser.add_argument("filterDisplay", type=str)
parser.add_argument("active", type=str)
parser.add_argument("nextPage", type=str, location="args")
parser.add_argument("primaryEmail", type=str, location="args")
parser.add_argument("filterDisplay", type=str, location="args")
parser.add_argument("active", type=str, location="args")

args = parser.parse_args()

Expand Down
Expand Up @@ -88,7 +88,7 @@ def test_returning_query_by_any_staff_only_active_true(self, fake_jwks):
follow_redirects=True,
)

logger.info("All staff users returned.")
logger.info("All staff users returned.", result.json)
assert result.json["users"] is not None

if result.json["nextPage"]:
Expand Down

0 comments on commit 4aa97e3

Please sign in to comment.