Skip to content

Commit

Permalink
test: Reorganize backup/restore tests for speed and reliability (#3537)
Browse files Browse the repository at this point in the history
We should do the backup/restore tests _after_ we do the basic tests. This is both more efficient as we avoid an extra up/down cycle and more meaningful as we will back up and restore an actually used system.

A bit hard to measure directly as this also moves the initial `docker compose up -w` into the test suite but a random run without this patch took about 10m 49s to finish for the testing part whereas with the patch it came down to 9m 10s so **almost 2 minutes faster**!
  • Loading branch information
BYK authored Jan 16, 2025
1 parent c075cf5 commit 63b6c0a
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 25 deletions.
5 changes: 5 additions & 0 deletions _integration-test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@

@pytest.fixture(scope="session", autouse=True)
def configure_self_hosted_environment(request):
subprocess.run(
["docker", "compose", "--ansi", "never", "up", "--wait"],
check=True,
capture_output=True,
)
# Create test user
subprocess.run(
[
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def test_sentry_admin(setup_backup_restore_env_variables):
assert "Usage: ./sentry-admin.sh permissions" in output


def test_backup(setup_backup_restore_env_variables):
def test_01_backup(setup_backup_restore_env_variables):
# Docker was giving me permission issues when trying to create this file and write to it even after giving read + write access
# to group and owner. Instead, try creating the empty file and then give everyone write access to the backup file
file_path = os.path.join(os.getcwd(), "sentry", "backup.json")
Expand All @@ -41,19 +41,22 @@ def test_backup(setup_backup_restore_env_variables):
assert os.path.getsize(file_path) > 0


def test_import(setup_backup_restore_env_variables):
def test_02_import(setup_backup_restore_env_variables):
# Bring postgres down and recreate the docker volume
subprocess.run(["docker", "compose", "--ansi", "never", "down"], check=True)
# We reset all DB-related volumes here and not just Postgres although the backups
# are only for Postgres. The reason is to get a "clean slate" as we need the Kafka
# and Clickhouse volumes to be back to their initial state as well ( without any events)
# We cannot just rm and create them as they still need migrations.
# and Clickhouse volumes to be back to their initial state as well (without any events)
# We cannot just rm and create them as they still need the migrations.
for name in ("postgres", "clickhouse", "kafka"):
subprocess.run(["docker", "volume", "rm", f"sentry-{name}"], check=True)
subprocess.run(["docker", "volume", "create", f"sentry-{name}"], check=True)
subprocess.run(
[
"rsync",
"-aW",
"--super",
"--numeric-ids",
"--no-compress",
"--mkpath",
join(os.environ["RUNNER_TEMP"], "volumes", f"sentry-{name}", ""),
Expand All @@ -62,24 +65,6 @@ def test_import(setup_backup_restore_env_variables):
check=True,
capture_output=True,
)
subprocess.run(["docker", "volume", "create", f"sentry-{name}"], check=True)

subprocess.run(
[
"docker",
"run",
"--rm",
"-v",
"sentry-kafka:/data",
"busybox",
"chown",
"-R",
"1000:1000",
"/data",
],
check=True,
capture_output=True,
)

subprocess.run(
["docker", "compose", "--ansi", "never", "up", "--wait"],
Expand All @@ -97,3 +82,4 @@ def test_import(setup_backup_restore_env_variables):
],
check=True,
)
# TODO: Check something actually restored here like the test user we have from earlier
3 changes: 1 addition & 2 deletions action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,12 @@ runs:
shell: bash
run: |
sudo chown root /usr/bin/rsync && sudo chmod u+s /usr/bin/rsync
rsync -aW --no-compress --mkpath \
rsync -aW --super --numeric-ids --no-compress --mkpath \
/var/lib/docker/volumes/sentry-postgres \
/var/lib/docker/volumes/sentry-clickhouse \
/var/lib/docker/volumes/sentry-kafka \
"$RUNNER_TEMP/volumes/"
cd ${{ github.action_path }}
docker compose up --wait
pytest -x --cov --junitxml=junit.xml _integration-test/
- name: Upload coverage to Codecov
Expand Down
3 changes: 2 additions & 1 deletion sentry-admin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ on the host filesystem. Commands that write files should write them to the '/sen

# Actual invocation that runs the command in the container.
invocation() {
$dcr --quiet-pull -v "$VOLUME_MAPPING" -T -e SENTRY_LOG_LEVEL=CRITICAL web "$@" 2>&1
$dc up postgres --wait
$dcr --no-deps --quiet-pull -v "$VOLUME_MAPPING" -T -e SENTRY_LOG_LEVEL=CRITICAL web "$@" 2>&1
}

# Function to modify lines starting with `Usage: sentry` to say `Usage: ./sentry-admin.sh` instead.
Expand Down

0 comments on commit 63b6c0a

Please sign in to comment.