Skip to content

Commit 63b6c0a

Browse files
authored
test: Reorganize backup/restore tests for speed and reliability (#3537)
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**!
1 parent c075cf5 commit 63b6c0a

File tree

5 files changed

+16
-25
lines changed

5 files changed

+16
-25
lines changed

_integration-test/conftest.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@
1212

1313
@pytest.fixture(scope="session", autouse=True)
1414
def configure_self_hosted_environment(request):
15+
subprocess.run(
16+
["docker", "compose", "--ansi", "never", "up", "--wait"],
17+
check=True,
18+
capture_output=True,
19+
)
1520
# Create test user
1621
subprocess.run(
1722
[
File renamed without changes.

_integration-test/test_backup.py renamed to _integration-test/test_02_backup.py

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def test_sentry_admin(setup_backup_restore_env_variables):
2020
assert "Usage: ./sentry-admin.sh permissions" in output
2121

2222

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

4343

44-
def test_import(setup_backup_restore_env_variables):
44+
def test_02_import(setup_backup_restore_env_variables):
4545
# Bring postgres down and recreate the docker volume
4646
subprocess.run(["docker", "compose", "--ansi", "never", "down"], check=True)
4747
# We reset all DB-related volumes here and not just Postgres although the backups
4848
# are only for Postgres. The reason is to get a "clean slate" as we need the Kafka
49-
# and Clickhouse volumes to be back to their initial state as well ( without any events)
50-
# We cannot just rm and create them as they still need migrations.
49+
# and Clickhouse volumes to be back to their initial state as well (without any events)
50+
# We cannot just rm and create them as they still need the migrations.
5151
for name in ("postgres", "clickhouse", "kafka"):
5252
subprocess.run(["docker", "volume", "rm", f"sentry-{name}"], check=True)
53+
subprocess.run(["docker", "volume", "create", f"sentry-{name}"], check=True)
5354
subprocess.run(
5455
[
5556
"rsync",
5657
"-aW",
58+
"--super",
59+
"--numeric-ids",
5760
"--no-compress",
5861
"--mkpath",
5962
join(os.environ["RUNNER_TEMP"], "volumes", f"sentry-{name}", ""),
@@ -62,24 +65,6 @@ def test_import(setup_backup_restore_env_variables):
6265
check=True,
6366
capture_output=True,
6467
)
65-
subprocess.run(["docker", "volume", "create", f"sentry-{name}"], check=True)
66-
67-
subprocess.run(
68-
[
69-
"docker",
70-
"run",
71-
"--rm",
72-
"-v",
73-
"sentry-kafka:/data",
74-
"busybox",
75-
"chown",
76-
"-R",
77-
"1000:1000",
78-
"/data",
79-
],
80-
check=True,
81-
capture_output=True,
82-
)
8368

8469
subprocess.run(
8570
["docker", "compose", "--ansi", "never", "up", "--wait"],
@@ -97,3 +82,4 @@ def test_import(setup_backup_restore_env_variables):
9782
],
9883
check=True,
9984
)
85+
# TODO: Check something actually restored here like the test user we have from earlier

action.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,13 +111,12 @@ runs:
111111
shell: bash
112112
run: |
113113
sudo chown root /usr/bin/rsync && sudo chmod u+s /usr/bin/rsync
114-
rsync -aW --no-compress --mkpath \
114+
rsync -aW --super --numeric-ids --no-compress --mkpath \
115115
/var/lib/docker/volumes/sentry-postgres \
116116
/var/lib/docker/volumes/sentry-clickhouse \
117117
/var/lib/docker/volumes/sentry-kafka \
118118
"$RUNNER_TEMP/volumes/"
119119
cd ${{ github.action_path }}
120-
docker compose up --wait
121120
pytest -x --cov --junitxml=junit.xml _integration-test/
122121
123122
- name: Upload coverage to Codecov

sentry-admin.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ on the host filesystem. Commands that write files should write them to the '/sen
2222

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

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

0 commit comments

Comments
 (0)