Add CI tests

.github/actions/run-docker/action.yml

@@ -15,8 +15,8 @@ inputs:
   logs:
     description: 'Show logs'
     required: false
-  data_backup_skip:
-    description: 'Skip data backup'
+  initialize:
+    description: 'Run the initialize script'
     required: false
     default: 'true'
   target:
@@ -36,6 +36,8 @@ runs:
   using: 'composite'
   steps:
     - name: Run Docker Container
+      id: run
+      continue-on-error: true
       shell: bash
       run: |
         # Start the specified services
@@ -52,11 +54,13 @@ runs:
 
         # Exec the run command in the container
         # quoted 'EOF' to prevent variable expansion
-        cat <<'EOF' | docker compose exec --user olympia web sh
+        cat <<'EOF' | docker compose exec --user olympia web bash
           ${{ inputs.run }}
         EOF
 
     - name: Logs
-      if: ${{ inputs.logs }}
       shell: bash
-      run: docker compose logs
+      if: ${{ steps.run.outcome == 'failure' }}
+      run: |
+        docker compose logs
+        exit 1

Makefile-docker

@@ -69,7 +69,7 @@ data_dump:
 
 .PHONY: data_load
 data_load:
-	./manage.py data_load $(ARGS)
+	./manage.py data_load --name $(LOAD)
 
 .PHONY: update_assets
 update_assets:

Makefile-os

@@ -18,32 +18,38 @@ export DATA_BACKUP_SKIP ?=
 override DOCKER_MYSQLD_VOLUME = addons-server_data_mysqld
 
 INITIALIZE_ARGS ?=
-INIT_CLEAN ?=
-INIT_LOAD ?=
-
-ifneq ($(INIT_CLEAN),)
-	INITIALIZE_ARGS += --clean
-endif
-
-ifneq ($(INIT_LOAD),)
-	INITIALIZE_ARGS += --load $(INIT_LOAD)
-endif
+export CLEAN ?=
+export LOAD ?=
 
 DOCKER_BAKE_ARGS := \
 	--file docker-bake.hcl \
 	--file .env \
 	--progress $(DOCKER_PROGRESS) \
 	--metadata-file $(DOCKER_METADATA_FILE) \
 
-ifeq ($(DOCKER_PUSH), true)
-	DOCKER_BAKE_ARGS += --push
-endif
-
 DOCKER_COMPOSE_ARGS := \
 	-d \
 	--remove-orphans \
 	--no-build \
 	--quiet-pull \
+	--timestamps
+
+ifneq ($(CLEAN),)
+	# Build without cache
+	DOCKER_COMPOSE_ARGS += --no-cache
+	# Force recreate all containers
+	DOCKER_COMPOSE_ARGS += --force-recreate
+	# Clean the database and all other data when Initializing
+	INITIALIZE_ARGS += --clean
+endif
+
+ifneq ($(LOAD),)
+	INITIALIZE_ARGS += --load $(LOAD)
+endif
+
+ifeq ($(DOCKER_PUSH), true)
+	DOCKER_BAKE_ARGS += --push
+endif
 
 ifneq ($(DOCKER_WAIT),)
 	DOCKER_COMPOSE_ARGS += --wait

docker-compose.yml

@@ -163,10 +163,14 @@ services:
 
   redis:
     image: redis:6.2
+    volumes:
+      - data_redis:/data
 
   rabbitmq:
     image: rabbitmq:3.12
     hostname: olympia
+    volumes:
+      - data_rabbitmq:/var/lib/rabbitmq
     expose:
       - "5672"
     environment:

docs/topics/development/data_management.md

@@ -27,18 +27,18 @@ But there are a few additional edge cases that it supports.
 ### Clean the database
 
   ```sh
-  make initialize INIT_CLEAN=true
+  make initialize CLEAN=true
   ```
 
   This will force the database to be recreated, and re-initialized.
 
 ### Load a data backup
 
   ```sh
-  make initialize [INIT_LOAD=<backup_name>]
+  make initialize [LOAD=<backup_name>]
   ```
 
-  This command will load a data backup from a specified path. The optional `INIT_LOAD` argument allows you to
+  This command will load a data backup from a specified path. The optional `LOAD` argument allows you to
   specify the path to the data backup file. If not specified, the initialize command will determine if
   data should be loaded based on the current state of the databse, and will load the `_init` data backup.
 

scripts/setup.py

@@ -11,27 +11,38 @@ def set_env_file(values):
             print(f'{key}={value}')
 
 
-def get_env_file():
+def get_env_file(path='.env'):
     env = {}
 
-    if os.path.exists('.env'):
-        with open('.env', 'r') as f:
+    if os.path.exists(path):
+        with open(path, 'r') as f:
             for line in f:
                 key, value = line.strip().split('=', 1)
-                env[key] = value.strip('"')
+                if value.startswith('"') and value.endswith('"'):
+                    value = value[1:-1]
+                env[key] = value
     return env
 
 
-def get_value(key, default_value):
-    if key in os.environ:
-        return os.environ[key]
+def get_value(key, default_value, from_file=True):
+    value = default_value
+    # Try to read the value from the .env file
+    # if we are allowed to do so
+    if from_file:
+        env = get_env_file()
+        if key in env:
+            value = env[key]
 
-    from_file = get_env_file()
+    # Always allow environment to override
+    # the default value
+    if key in os.environ:
+        value = os.environ[key]
 
-    if key in from_file:
-        return from_file[key]
+    # Do not allow empty or None values
+    if value is not None and value != '':
+        return value
 
-    return default_value
+    raise ValueError(f'{key} is not set')
 
 
 def get_docker_tag():

src/olympia/core/apps.py

@@ -53,12 +53,22 @@ def host_check(app_configs, **kwargs):
     actual_uid = getpwnam('olympia').pw_uid
 
     if actual_uid != expected_uid:
-        return [
+        errors.append(
             Error(
                 f'Expected user uid to be {expected_uid}, received {actual_uid}',
                 id='setup.E002',
             )
-        ]
+        )
+
+    if (
+        settings.HOST_MOUNT is None or settings.HOST_MOUNT == 'production'
+    ) and os.path.exists('/data/olympia/Makefile-os'):
+        errors.append(
+            Error(
+                'Makefile-os should be excluded by dockerignore',
+                id='setup.E003',
+            )
+        )
 
     return errors
 

tests/make/test_setup.py

@@ -19,8 +19,10 @@
 
 class BaseTestClass(unittest.TestCase):
     def assert_set_env_file_called_with(self, **kwargs):
-        expected = {key: kwargs.get(key, mock.ANY) for key in keys}
-        assert mock.call(expected) in self.mock_set_env_file.call_args_list
+        actual = self.mock_set_env_file.call_args_list[0].args[0]
+        for key in kwargs:
+            assert key in actual.keys()
+            assert actual.get(key) == kwargs.get(key, mock.ANY)
 
     def setUp(self):
         patch = mock.patch('scripts.setup.set_env_file')
@@ -32,6 +34,101 @@ def setUp(self):
         self.mock_get_env_file = patch_two.start()
 
 
+class TestBaseTestClass(BaseTestClass):
+    def test_invalid_key_raises(self):
+        with self.assertRaises(AssertionError):
+            main()
+            self.assert_set_env_file_called_with(FOO=True)
+
+
+@override_env()
+class TestGetEnvFile(BaseTestClass):
+    def setUp(self):
+        import tempfile
+
+        self.tmp_dir = tempfile.mkdtemp()
+        self.path = os.path.join(self.tmp_dir, 'test_env')
+
+    def tearDown(self):
+        import shutil
+
+        shutil.rmtree(self.tmp_dir)
+
+    def _write_and_assert(self, value, expected, write_value=True):
+        if write_value:
+            with open(self.path, 'w') as f:
+                f.write(f'key="{value}"')
+        expected_value = {'key': expected} if type(expected) is str else expected
+        self.assertEqual(get_env_file(self.path), expected_value)
+
+    def test_get_env_file_missing(self):
+        self._write_and_assert('value', {}, write_value=False)
+
+    def test_get_value_default(self):
+        self._write_and_assert('value', 'value')
+
+    def test_get_empty_value(self):
+        self._write_and_assert('', '')
+
+    def test_get_quoted_value(self):
+        self._write_and_assert('"quoted_value"', '"quoted_value"')
+
+    def test_get_single_quoted_value(self):
+        self._write_and_assert("'quoted_value'", "'quoted_value'")
+
+    def test_get_unmatched_quotes(self):
+        self._write_and_assert('"unmatched_quote', '"unmatched_quote')
+
+    def test_get_nested_quotes(self):
+        self._write_and_assert(
+            'value with "nested" quotes', 'value with "nested" quotes'
+        )
+
+
+class TestGetValue(BaseTestClass):
+    @override_env()
+    def test_get_value_from_default(self):
+        self.mock_get_env_file.return_value = {}
+        self.assertEqual(get_value('TEST_KEY', 'default'), 'default')
+
+    @override_env()
+    def test_get_value_from_default_with_invalid_default(self):
+        self.mock_get_env_file.return_value = {}
+        for value in [None, '']:
+            with self.assertRaises(ValueError):
+                get_value('TEST_KEY', value)
+
+    @override_env()
+    def test_get_value_from_file(self):
+        self.mock_get_env_file.return_value = {'TEST_KEY': 'file_value'}
+        self.assertEqual(get_value('TEST_KEY', 'default'), 'file_value')
+
+    @override_env()
+    def test_get_value_from_file_invalid_value(self):
+        for value in [None, '']:
+            self.mock_get_env_file.return_value = {'TEST_KEY': value}
+            with self.assertRaises(ValueError):
+                get_value('TEST_KEY', value)
+
+    @override_env(TEST_KEY='env_value')
+    def test_get_value_from_env(self):
+        self.assertEqual(get_value('TEST_KEY', 'default'), 'env_value')
+
+    @override_env(TEST_KEY='env_value')
+    def test_get_value_from_env_overrides_file(self):
+        self.mock_get_env_file.return_value = {'TEST_KEY': 'file_value'}
+        self.assertEqual(get_value('TEST_KEY', 'default'), 'env_value')
+
+    def test_get_value_from_env_invalid_value(self):
+        with override_env(TEST_KEY=''):
+            with self.assertRaises(ValueError):
+                get_value('TEST_KEY', None)
+
+        with override_env():
+            with self.assertRaises(ValueError):
+                get_value('TEST_KEY', None)
+
+
 @override_env()
 class TestGetDockerTag(BaseTestClass):
     def test_default_value_is_local(self):