respond to comments and fix tests

Makefile

@@ -71,7 +71,7 @@ api/test-focus:			## Run api python tests marked with `@pytest.mark.focus`
 	@cd api/; . .venv/bin/activate; pytest test/api -vv -s -m focus
 
 api/test-e2e:			## Run api python e2e tests
-	@cd api/; . .venv/bin/activate; pytest test/e2e -v
+	@cd api/; . .venv/bin/activate; pytest test/e2e -v  --order-scope=module 
 
 api/test-e2e-focus:		## Run api python e2e tests marked with `@pytest.mark.focus`
 	@cd api/; . .venv/bin/activate; pytest test/e2e -v -s -m focus

api/requirements.txt

@@ -14,4 +14,5 @@ python-dotenv
 python-multipart
 uvicorn
 requests
-strenum
+strenum
+pytest-order

api/test/e2e/journeys/base_journey.py

@@ -10,7 +10,7 @@
 import pandas as pd
 from io import StringIO
 from uuid import uuid4
-from strenum import StrEnum
+from enum import Enum
 from jinja2 import Template
 
 from api.common.config.constants import CONTENT_ENCODING
@@ -21,9 +21,14 @@
 FILE_PATH = os.path.dirname(os.path.abspath(__file__))
 
 
-class SchemaVersion(StrEnum):
+class SchemaVersion(Enum):
     V1 = "v1"
     V2 = "v2"
+    V3 = "v3"
+
+    def __str__(self):
+        # Override the string representation to return the value directly
+        return self.value
 
 
 class BaseJourneyTest(ABC):
@@ -119,7 +124,6 @@ def generate_auth_headers(self, client_name: str = None) -> dict:
         if not client_name:
             client_name = self.client_name()
 
-        # TODO: Can this be cached to reduce the amount of calls?
         token_url = f"https://{DOMAIN_NAME}/api/oauth2/token"
         data_admin_credentials = get_secret(
             secret_name=f"{RESOURCE_PREFIX}_{client_name}"  # pragma: allowlist secret
@@ -181,31 +185,6 @@ def delete_dataset(cls, name: str) -> str:
         )
         assert response.status_code == HTTPStatus.ACCEPTED
 
-    @classmethod
-    def delete_user(cls, username: str, user_id) -> str:
-        response = requests.delete(
-            f"{cls.user_url(cls)}",
-            headers=cls.generate_auth_headers(cls, "E2E_TEST_CLIENT_USER_ADMIN"),
-            data=json.dumps({"username": username, "user_id": user_id}),
-        )
-        assert response.status_code == HTTPStatus.ACCEPTED
-
-    @classmethod
-    def delete_client(cls, client_id: str) -> str:
-        response = requests.delete(
-            f"{cls.client_url(cls)}/{client_id}",
-            headers=cls.generate_auth_headers(cls, "E2E_TEST_CLIENT_USER_ADMIN"),
-        )
-        assert response.status_code == HTTPStatus.ACCEPTED
-
-    @classmethod
-    def delete_domain(cls, name: str) -> str:
-        response = requests.delete(
-            cls.protected_domain_url(cls, name),
-            headers=cls.generate_auth_headers(cls, "E2E_TEST_CLIENT_DATA_ADMIN"),
-        )
-        assert response.status_code == HTTPStatus.ACCEPTED
-
     def generate_username_payload(self) -> dict:
         unique_id = uuid.uuid4()
         # Append the UUID to the username and email

api/test/e2e/journeys/schemas/test_e2e-schema_generated_schema.json.tpl

@@ -0,0 +1,63 @@
+{
+  "metadata": {
+    "layer": "default",
+    "domain": "test_e2e",
+    "dataset": "{{ name }}",
+    "sensitivity": "PUBLIC",
+    "description": "",
+    "key_value_tags": {},
+    "key_only_tags": [],
+    "owners": [
+      {
+        "name": "change_me",
+        "email": "[email protected]"
+      }
+    ],
+    "update_behaviour": "APPEND",
+    "is_latest_version": true
+  },
+  "columns": [
+    {
+      "name": "year",
+      "data_type": "int",
+      "allow_null": true,
+      "partition_index": null,
+      "format":null
+    },
+    {
+      "name": "month",
+      "data_type": "int",
+      "allow_null": true,
+      "partition_index": null,
+      "format":null
+    },
+    {
+      "name": "destination",
+      "data_type": "string",
+      "allow_null": true,
+      "partition_index": null,
+      "format":null
+    },
+    {
+      "name": "arrival",
+      "data_type": "string",
+      "allow_null": true,
+      "partition_index": null,
+      "format":null
+    },
+    {
+      "name": "type",
+      "data_type": "string",
+      "allow_null": true,
+      "partition_index": null,
+      "format":null
+    },
+    {
+      "name": "status",
+      "data_type": "string",
+      "allow_null": true,
+      "partition_index": null,
+      "format":null
+    }
+  ]
+}

api/test/e2e/journeys/test_datasets.py

@@ -13,6 +13,7 @@
 
 RESOURCE_PREFIX = os.environ["RESOURCE_PREFIX"]
 
+
 class TestDataJourneys(BaseAuthenticatedJourneyTest):
     dataset = None
 
@@ -201,62 +202,63 @@ def test_lists_raw_datasets(self):
         )
         assert available_datasets_response.status_code == HTTPStatus.OK
         filenames = available_datasets_response.json()
-        assert len(filenames) == 2 # 2 files uploaded csv and parquet
+        assert len(filenames) == 2  # 2 files uploaded csv and parquet
 
     @pytest.mark.order(2)
-    def test_upload_to_invalid_location(self):
+    @pytest.mark.parametrize(
+        "layer, domain, dataset, expected_status",
+        [
+            (
+                "invalid-layer",
+                "test_e2e",
+                "test_dataset",
+                HTTPStatus.BAD_REQUEST,
+            ),
+            (
+                "default-layer",
+                "invalid-domain",
+                "test_dataset",
+                HTTPStatus.BAD_REQUEST,
+            ),
+            (
+                "default-layer",
+                "test_e2e",
+                "invalid-dataset",
+                HTTPStatus.BAD_REQUEST,
+            ),
+        ],
+    )
+    def test_upload_to_invalid_location(self, layer, domain, dataset, expected_status):
         files = {
             "file": (self.csv_filename, open("./test/e2e/" + self.csv_filename, "rb"))
         }
-
-        invalid_params = {
-            "layer": "invalid-layer",
-            "domain": "invalid-domain",
-            "dataset": "invalid-dataset"
-        }
-
-        invalid_response_codes = []
-        for k,v in invalid_params.items():
-            if k == "layer":
-                upload_url = self.upload_dataset_url(
-                    v, self.e2e_test_domain, self.dataset
-                )
-                assert self.return_status_code_for_invalid_url(upload_url) == HTTPStatus.BAD_REQUEST
-            elif k == "domain":
-                upload_url = self.upload_dataset_url(
-                    self.layer, v, self.dataset
-                )
-                assert self.return_status_code_for_invalid_url(upload_url) == HTTPStatus.NOT_FOUND
-            elif k == "dataset":
-                upload_url = self.upload_dataset_url(
-                    self.layer, self.e2e_test_domain, v
-                )
-                assert self.return_status_code_for_invalid_url(upload_url) == HTTPStatus.NOT_FOUND
+        upload_url = self.upload_dataset_url(layer, domain, dataset)
+        assert self.return_status_code_for_invalid_url(upload_url) == expected_status
 
     @pytest.mark.order(2)
     def test_large_data_endpoint(self):
-        large_dataset_url = f"{self.query_dataset_url_large(
+        large_dataset_url = self.query_dataset_url_large(
             layer=self.layer,
             domain=self.e2e_test_domain,
             dataset=self.dataset,
-            version=1
-        )}"
+            version=1,
+        )
 
-        response = requests.post(large_dataset_url, headers=self.generate_auth_headers())
+        response = requests.post(
+            large_dataset_url, headers=self.generate_auth_headers()
+        )
         assert response.status_code == HTTPStatus.ACCEPTED
-        job_id = response.json()['details']["job_id"]
+        job_id = response.json()["details"]["job_id"]
         job_id_url = f"{self.jobs_url()}/{job_id}"
 
         response = requests.get(job_id_url, headers=self.generate_auth_headers())
         assert response.status_code == HTTPStatus.OK
 
-    #TODO: Why does this need authentication?
     def test_always_list_layers(self):
         response = requests.get(
             self.layers_url(),
             headers=self.generate_auth_headers(),
         )
-        print(response.content)
         assert response.status_code == HTTPStatus.OK
 
     @pytest.mark.order(3)
@@ -273,4 +275,4 @@ def delete_existing_dataset(self):
             delete_raw_data_url, headers=self.generate_auth_headers()
         )
 
-        assert response.status_code == HTTPStatus.OK
+        assert response.status_code == HTTPStatus.OK

api/test/e2e/journeys/test_protected_domains.py

@@ -41,7 +41,6 @@ def setup_class(cls):
         # Create schema
         cls.dataset = cls.create_schema("protected")
 
-        # TODO: Discuss if this should actually be in the journey itself?
         # Upload file
         files = {
             "file": (cls.csv_filename, open("./test/e2e/" + cls.csv_filename, "rb"))
@@ -84,17 +83,14 @@ def assume_permissions(self, permissions: List[str]):
     def reset_permissions(self):
         self.assume_permissions([])
 
-    # We have these orders because the datasets and domain tests interfere.
-    @pytest.mark.order(4)
+    @pytest.mark.order(1)
     def test_create_protected_domain(self):
         self.reset_permissions()
         # Create protected domain
         # Generate a random string (UUID without hyphens)
-        print("DOMAIN NAME:", self.test_domain_name)
         create_url = self.protected_domain_url(self.test_domain_name)
 
         response = requests.post(create_url, headers=self.generate_auth_headers())
-        print("RESPONSE CONTENT", response.content)
         assert response.status_code == HTTPStatus.CREATED
 
         # Lists created protected domain
@@ -109,7 +105,7 @@ def test_create_protected_domain(self):
         response = requests.post(url, headers=self.generate_auth_headers())
         assert response.status_code == HTTPStatus.UNAUTHORIZED
 
-    @pytest.mark.order(5)
+    @pytest.mark.order(2)
     def test_allows_access_to_protected_domain_when_granted_permission(self):
         self.assume_permissions(["READ_DEFAULT_PROTECTED_TEST_E2E_PROTECTED"])
 
@@ -138,12 +134,11 @@ def test_allows_access_to_protected_domain_when_granted_permission(self):
 
         self.reset_permissions()
 
-    @pytest.mark.order(6)
+    @pytest.mark.order(3)
     def test_delete_protected_domain(self):
         # Delete protected domain
         delete_url = self.protected_domain_url(self.test_domain_name)
         response = requests.delete(delete_url, headers=self.generate_auth_headers())
-        print(response.content)
         assert response.status_code == HTTPStatus.ACCEPTED
         # Lists protected domain and checks it's gone
         list_url = self.list_protected_domain_url()