Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration tests: don't hard-code MySQL in some tests #709

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

julianbrost
Copy link
Contributor

@julianbrost julianbrost commented Mar 18, 2024

Some test cases always used a MySQL database as they didn't use the proper function which would respect the ICINGADB_TESTS_DATABASE_TYPE=pgsql environment variable. This commit fixes this and updates a similarly left-over error message as well.

Tests

Excerpts from the GitHub Actions job artifacts (xzgrep -E 'created (mysql|postgresql) container'):

Current main branch

https://github.com/Icinga/icingadb/actions/runs/8324491540

mysql-debug.log.xz

✔️ No problem, creates only a MySQL container as expected:

{"L":"DEBUG","T":"2024-03-18T09:52:05.779Z","M":"created mysql container","mysql":true,"container-name":"icinga-testing-62f6a821-mysql","container-id":"567aae686d80883b356a20e7340e553a3f97a8c87a94819a0af242f5ac8c7367"}

pgsql-debug.log.xz

❌ Unexpectedly creates a MySQL container in addition to the expected PostgreSQL container:

{"L":"DEBUG","T":"2024-03-18T09:52:03.145Z","M":"created postgresql container","postgresql":true,"container-name":"icinga-testing-52b5911a-postgresql","container-id":"908a1477ca014e370f55b086c2c5ce3018a74b8eec62c8fdb2254243201953be"}
{"L":"DEBUG","T":"2024-03-18T10:01:04.911Z","M":"created mysql container","mysql":true,"container-name":"icinga-testing-52b5911a-mysql","container-id":"b07bb30672c3403ad71b841afd3633cd1bb5a72522bfd2b81f56bc9d911689bc"}

This PR

https://github.com/Icinga/icingadb/actions/runs/8329451180?pr=709

mysql-debug.log.xz

✔️ Still only a MySQL container, so still no problem:

{"L":"DEBUG","T":"2024-03-18T15:41:43.526Z","M":"created mysql container","mysql":true,"container-name":"icinga-testing-b8dbc604-mysql","container-id":"6c56e976188d9e0152bbe9e331155e1d4148efd4777f231e84a84c3ac720fe3d"}

pgsql-debug.log.xz

✔️ Now fixed, no longer creates a MySQL container but just the expected PostgreSQL container:

{"L":"DEBUG","T":"2024-03-18T15:41:44.779Z","M":"created postgresql container","postgresql":true,"container-name":"icinga-testing-a4dca8d8-postgresql","container-id":"90f50752265b61a3d2424267a115033a099c21a962c16ae0e01a5cf392745b54"}

Benchmarks

The benchmark doesn't run automatically in GitHub Actions. When running locally, it currently fails with the following error:

BenchmarkHistory/100000-Comments
    history_bench_test.go:82: 
        	Error Trace:	/home/jbrost/src/icingadb/tests/history_bench_test.go:82
        	            				/home/jbrost/src/icingadb/tests/history_bench_test.go:93
        	            				/home/jbrost/src/icingadb/tests/history_bench_test.go:22
        	            				/usr/lib/go/src/testing/benchmark.go:193
        	            				/usr/lib/go/src/testing/benchmark.go:215
        	            				/usr/lib/go/src/runtime/asm_amd64.s:1695
        	Error:      	Received unexpected error:
        	            	redis: got 20 elements in XINFO STREAM reply,wanted 14
        	Test:       	BenchmarkHistory/100000-Comments
        	Messages:   	XINFO should not fail
--- FAIL: BenchmarkHistory/100000-Comments

That's an error from the Redis client library we use as it fails to parse a protocol message. I believe this happens due to the following change to XINFO STREAM command in Redis 7.0:

Starting with Redis version 7.0.0: Added the max-deleted-entry-id, entries-added, recorded-first-entry-id, entries-read and lag fields

When running with ICINGA_TESTING_REDIS_IMAGE=redis:6, this error doesn't happen, the benchmark still doesn't work though, now it just stays at all entries pending in the stream, but that's doesn't seem to be a PostgreSQL-specific problem as it happens on MySQL as well. So the PR should fix the hard-coded MySQL problem, but a different problem remains.

We currently use an outdated version of go-redis as the new version wasn't picked up by depandabot. I've created #708 for that.

Some test cases always used a MySQL database as they didn't use the proper
function which would respect the ICINGADB_TESTS_DATABASE_TYPE=pgsql environment
variable. This commit fixes this and updates a similarly left-over error
message as well.
@cla-bot cla-bot bot added the cla/signed label Mar 18, 2024
@julianbrost julianbrost added this to the 1.1.2 milestone Mar 18, 2024
@julianbrost julianbrost marked this pull request as ready for review March 18, 2024 16:05
@julianbrost julianbrost requested a review from lippserd March 18, 2024 16:05
@lippserd lippserd merged commit 6876f57 into main Mar 19, 2024
31 checks passed
@lippserd lippserd deleted the integration-tests-hard-coded-mysql branch March 19, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants