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

Fix DB unit tests #3186

Merged
merged 24 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
86127f9
add docker compose that allows all services to run locally
fhanik Dec 9, 2024
fd5adcc
Fix flakiness in a test that does not yield time change on a fast system
fhanik Dec 9, 2024
ee70778
tests: fix unit tests to support all three databases
fhanik Dec 10, 2024
3a8d0c6
tests: use java initializer in TableAndColumnNormalizationTest
Kehrlann Dec 4, 2024
b81e0cc
tests: introduce @EnabledIfProfile / @DisabledIfProfile
Kehrlann Dec 9, 2024
80de45e
tests: fix hsql-specific JdbcPagingListTests
Kehrlann Dec 9, 2024
0596320
tests: fix JdbcScimUserProvisioningTests.retrieveByScimFilter_Include…
Kehrlann Dec 10, 2024
2ca7f14
tests: fix JdbcClientMetadataProvisioningTest.createdByPadsTo36Chars …
Kehrlann Dec 10, 2024
2ae2439
Remove hard coded database values from the MockMvc integration tests
fhanik Dec 10, 2024
c0162f5
Downgrade MySQL to avoid padding issues
fhanik Dec 10, 2024
97f475a
Remove database connections leaks (1 prod, many in tests)
fhanik Dec 11, 2024
5d33664
Change table columns `users.id` to varchar from char
fhanik Dec 11, 2024
0eb383a
Ensure that the table `groups` is properly escaped since groups is a …
fhanik Dec 11, 2024
49f7a30
add TODO in ScimUserEndpointsAliasMockMvcTests.AliasFeatureEnabled
fhanik Dec 11, 2024
4b02a1b
No more CHAR datatypes, only varchar
fhanik Dec 11, 2024
eece476
Fix mistake on column null handling
fhanik Dec 12, 2024
fea1a59
tests: disable BootstrapTests in mysql and postgresql profiles
Kehrlann Dec 13, 2024
03b544e
tests: remove duplicate test
Kehrlann Dec 16, 2024
34656be
tests: ensure mysql is on the same timezone as the host machine when …
Kehrlann Dec 16, 2024
9436ed4
Fix connection leak from Flyway
fhanik Dec 16, 2024
428b189
tests: database data volumes in docker-compose.yml uses tmpfs
Kehrlann Dec 17, 2024
de56eab
tests: re-enable createdByPadsTo36Chars excpet for MySQL
Kehrlann Dec 17, 2024
8738e29
docs: add docs/testing.md
Kehrlann Dec 17, 2024
345c7c1
tests: rework JdbcScimUserProvisioningTests.retrieveByScimFilter_Incl…
Kehrlann Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,14 +209,33 @@ You can run the integration tests with docker

will create a docker container running uaa + ldap + database whereby integration tests are run against.

### Using Gradle to test with postgresql or mysql
### Using Docker to test with postgresql or mysql

The default uaa unit tests (./gradlew test integrationTest) use hsqldb.

To run the unit tests with docker:

$ run-unit-tests.sh <dbtype>

### Using Gradle to test with Postgres or MySQL

You need a locally running database. You can launch a Postgres 15 and MySQL 8 locally with docker compose:

$ docker compose --file scripts/docker-compose.yaml up

If you wish to launch only one of the DBs, select the appropriate service name:

$ docker compose --file scripts/docker-compose.yaml up postgresql

Then run the test with the appropriate profile:

$ ./gradlew '-Dspring.profiles.active=postgresql,default' \
--no-daemon \
test

There are special guarantees in place to avoid pollution between tests, so be sure to run the images
from the compose script, and run your test with `--no-daemon`. To learn more, read [docs/testing.md](docs/testing.md).

### To run a single test

The default uaa unit tests (`./gradlew test`) use hsqldb.
Expand Down
136 changes: 136 additions & 0 deletions docs/testing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Testing considerations
Kehrlann marked this conversation as resolved.
Show resolved Hide resolved

This document contains information about how the tests are structured and why.

## Types of test

There are two types of tests:

- Unit tests, which run tests on classes or subsets of the application in a single JVM. Those run with `./gradlew test`.
- Integration tests, which launch the UAA application and run web-based tests the running app. Those can be run with
`./gradlew integrationTest`

## Helper scripts

There are helper scripts, `run-unit-tests.sh` and `run-integration-tests.sh`, which run the tests inside a docker
container. The docker container they run in contains the database against which to run the tests, as well as an LDAP
server. It is self-contained but lacks flexibility. It relies on custom-baked image that may not support arm64, and
can't work with your IDE.

However, since the scripts run a container with all dependencies, you do not need infrastructure to run against a
specified DB:

$ run-unit-tests.sh <dbtype>

## Test databases

By default, the tests run against an in-memory DB, `hsqldb`. This DB is also present in the prod artifact, so that
UAA can also be ran standalone to test tweaks in a live instance.

To run these databases locally, use the docker-compose script:

$ docker compose --file scripts/docker-compose.yaml up

If you wish to launch only one of the DBs, select the appropriate service name:

$ docker compose --file scripts/docker-compose.yaml up postgresql

To run tests against either Postgres or MySQL, use the `postgresql` or `mysql` profile, to select the DB. Be sure
to add the `default` profile which will trigger seeding the database with some admin users, clients, etc. For example:

Kehrlann marked this conversation as resolved.
Show resolved Hide resolved
$ ./gradlew '-Dspring.profiles.active=mysql,default test

To run tests from your IDE against a given database, you can (temporarily) annotate the test class:

```java

@ActiveProfiles({"mysql", "default"})
class MyCustomTests {
@Test
void foo() {
// ...
}
}
```

## Database-specific tests

Some tests only work on a single type of database, for example `MySqlDbMigrationIntegrationTest`; or do not work on a
given database, for example `JdbcClientMetadataProvisioningTest.createdByPadsTo36Chars`. You can turn tests on and off
based on the profile with custom annotations, `@DisabledIfProfile` and `@EnabledIfProfile`, for example:

```java
// Only run on either mysql or postgresql
@EnabledIfProfile({"mysql", "postgresql"})
class RealDbOnlyTests {

}

// or:

class SomeTests {

// Do not run when there is either mysql or postgresql
@DisabledIfProfile({"mysql", "postgres"})
void notOnRealDb() {

}

}
```

## Test pollution

There might be test pollution when tests are run in parallel, or even between projects. For example, when you run

$ ./gradlew test

It will run tests in both `cloudfoundry-identity-server` and `cloudfoundry-identity-uaa` projects. Both need a database,
and both do sometimes clean up the database.

To avoid test pollution, 24 databases are created, and each Gradle "worker" thread gets its own database. A Gradle
worker has a numeric `id`, and each time a new task is spun up, the idea of the worker picking up the task increases.
So there are 24 DBs with names `uaa_1`, `uaa_2`, ... created, and usually the worker ID stays below 24 and there are
enough databases for each test.

However, if the gradle daemon is kept running in the background and is re-used for subsequent tasks, e.g. by doing:

$ ./gradlew test # first run
# do some code changes
$ ./gradlew test

You will get new workers with IDs > 24. It is recommended you run your Gradle in no-daemon mode when running tests:

$ ./gradlew test --no-daemon

It will be slightly slower to start up (a few seconds), but the tests take multiple minutes and so the gain of using
a daemon is not worth the trouble.

## Timezone issues

The UAA and its DB server _MUST_ have the same timezone, because dates are not uniformly stored in UTC and timezones
do matter. Specifically for MySQL, there are issues when your local host is ahead of UTC, because:

1. The default containers runs in UTC
2. When calling `current_timestamp` the value is in UTC
3. But when calling a prepared statement from JDBC with a Date/Timestamp/time-based the timezone is sent to the server

So, when running e.g. in `Europe/Paris` (CET):
strehle marked this conversation as resolved.
Show resolved Hide resolved

```java
jdbcTemplate.queryForObject("SELECT CURRENT_TIMESTAMP",String .class);
// will return 15:00UTC
// if the TZ is dropped, it is recorded as 15:00
jdbcTemplate.update("UPDATE foo SET updated=?",new Date(System.currentTimeMillis()));
// will insert 16:00CET
// if the TZ is dropped this is recorded as 16:00
```

For this reason, we update the MySQL container in `docker-compose.yml` to have the same timezone as the host through
the `$TZ` env var.

If you have timing-based issues in your test, ensure that you set `$TZ` before starting docker compose, e.g.:

$ TZ="Europe/Paris" docker compose up

It is not required, and MySQL will default to using UTC.
50 changes: 50 additions & 0 deletions scripts/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
name: uaa

services:
postgres:
image: "postgres:15"
ports:
- 5432:5432
volumes:
- ./postgresql:/docker-entrypoint-initdb.d/
- type: tmpfs
target: /var/lib/postgresql/data
tmpfs:
size: 1073741824 # 1024 * 2^20 bytes = 1024Mb
environment:
- POSTGRES_PASSWORD=changeme
mysql:
image: "mysql:8"
ports:
- 3306:3306
volumes:
- ./mysql:/docker-entrypoint-initdb.d/
- /etc/localtime:/localtime-from-host
- type: tmpfs
target: /var/lib/mysql
tmpfs:
size: 1073741824 # 1024 * 2^20 bytes = 1024Mb
environment:
- MYSQL_ROOT_PASSWORD=changeme
- TZ=${TZ}
command:
- --sql_mode=ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION,PAD_CHAR_TO_FULL_LENGTH
openldap:
image: docker.io/bitnami/openldap:2.6
ports:
- '389:1389'
- '636:1636'
# docs of these env vars: https://github.com/bitnami/containers/tree/2724f9cd02b3b4e7986a1e2a0b0b30af3737bbd2/bitnami/openldap#configuration
environment:
- LDAP_ROOT=dc=test,dc=com
- LDAP_ADMIN_USERNAME=admin
- LDAP_ADMIN_PASSWORD=password
- LDAP_USERS=user01,user02
- LDAP_PASSWORDS=password1,password2
- LDAP_GROUP=some-ldap-group
volumes:
- 'openldap_data:/bitnami/openldap'

volumes:
openldap_data:
driver: local
50 changes: 50 additions & 0 deletions scripts/mysql/init-db.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env bash

set -euo pipefail

# Number of gradle workers times 4, which was somewhat arbitrary but is sufficient in practice.
# We make extra dbs because a gradle worker ID can exceed the max number of workers.
NUM_OF_DATABASES_TO_CREATE=24

function initDB() {
mysql -uroot -pchangeme <<-EOSQL
SET GLOBAL max_connections = 250;
DROP DATABASE IF EXISTS uaa;
CREATE DATABASE uaa DEFAULT CHARACTER SET utf8mb4;
EOSQL
}

function createDB() {
DATABASE_NAME="uaa_${1}"
echo "Creating MySQL database with name ${DATABASE_NAME}"
mysql -uroot -pchangeme <<-EOSQL
DROP DATABASE IF EXISTS $DATABASE_NAME;
CREATE DATABASE $DATABASE_NAME DEFAULT CHARACTER SET utf8mb4;
EOSQL
}

function setTimezone() {
# If the "TZ" env var is set in the container definition, then set the
# DB at the given timezone. When "TZ" is unset, use the DB default (UTC).
#
# This is important because the database should run in the same timezone as the UAA,
# and, in the case of tests, the same timezone as the JVM running the tests.
#
# We achieve consistency by changing the timezone inside the DB; because setting
# it in the container is complicated. The container is missing the `timedatectl`
# binary ; and the script runs as the mysql user which does not have sudo privileges.
if [[ -n "$TZ" ]]; then
echo "Setting DB timezone to: $TZ"
mysql -uroot -pchangeme <<-EOSQL
SET GLOBAL time_zone = "$TZ";
EOSQL
fi
}



initDB

for db_id in `seq 1 $NUM_OF_DATABASES_TO_CREATE`; do
createDB $db_id
done
32 changes: 32 additions & 0 deletions scripts/postgresql/init-db.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
#!/usr/bin/env bash

set -euo pipefail

# Number of gradle workers times 4, which was somewhat arbitrary but is sufficient in practice.
# We make extra dbs because a gradle worker ID can exceed the max number of workers.
NUM_OF_DATABASES_TO_CREATE=24

function initDB() {
psql --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
DROP DATABASE IF EXISTS uaa;
CREATE DATABASE uaa;
DROP USER IF EXISTS root;
CREATE USER root WITH SUPERUSER PASSWORD '$POSTGRES_PASSWORD';
SHOW max_connections;
EOSQL
}

function createDB() {
DATABASE_NAME="uaa_${1}"
echo "Creating PostgreSQL database with name ${DATABASE_NAME}"
psql --username "$POSTGRES_USER" --dbname "$POSTGRES_DB" <<-EOSQL
DROP DATABASE IF EXISTS $DATABASE_NAME;
CREATE DATABASE $DATABASE_NAME;
EOSQL
}

initDB

for db_id in `seq 1 $NUM_OF_DATABASES_TO_CREATE`; do
createDB $db_id
done
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata)
public Flyway flyway(Flyway baseFlyway) {
baseFlyway.repair();
baseFlyway.migrate();
org.apache.tomcat.jdbc.pool.DataSource ds =
strehle marked this conversation as resolved.
Show resolved Hide resolved
(org.apache.tomcat.jdbc.pool.DataSource)baseFlyway.getConfiguration().getDataSource();
ds.purge();
return baseFlyway;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
import org.flywaydb.core.api.migration.Context;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.jdbc.datasource.SingleConnectionDataSource;
import org.springframework.jdbc.datasource.lookup.DataSourceLookupFailureException;

import java.sql.Connection;
import java.sql.SQLException;


public class V4_9_2__AddPrimaryKeysIfMissing extends BaseJavaMigration {
Expand All @@ -16,11 +20,19 @@ public void migrate(Context context) throws Exception {
JdbcTemplate jdbcTemplate = new JdbcTemplate(new SingleConnectionDataSource(
context.getConnection(), true));
for (String table : tables) {
int count = jdbcTemplate.queryForObject(checkPrimaryKeyExists, Integer.class, jdbcTemplate.getDataSource().getConnection().getCatalog(), table);
int count = jdbcTemplate.queryForObject(checkPrimaryKeyExists, Integer.class, getDatabaseCatalog(jdbcTemplate), table);
if (count == 0) {
String sql = "ALTER TABLE " + table + " ADD COLUMN `id` int(11) unsigned PRIMARY KEY AUTO_INCREMENT";
jdbcTemplate.execute(sql);
}
}
}

private String getDatabaseCatalog(JdbcTemplate jdbcTemplate) {
try (Connection connection = jdbcTemplate.getDataSource().getConnection()) {
return connection.getCatalog();
} catch (SQLException e) {
throw new DataSourceLookupFailureException("Unable to look up database schema.", e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-- NOOP
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE users MODIFY id VARCHAR(36) NOT NULL;
Kehrlann marked this conversation as resolved.
Show resolved Hide resolved
ALTER TABLE oauth_client_details MODIFY created_by VARCHAR(36) NULL;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-- NOOP
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import org.cloudfoundry.identity.uaa.util.beans.PasswordEncoderConfig;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.context.annotation.ImportResource;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.context.web.WebAppConfiguration;
Expand All @@ -19,7 +18,6 @@
@Retention(RetentionPolicy.RUNTIME)
@ExtendWith(SpringExtension.class)
@ExtendWith(PollutionPreventionExtension.class)
@ActiveProfiles("default")
@WebAppConfiguration
@ContextConfiguration(classes = {
DatabaseOnlyConfiguration.class,
Expand Down
Loading
Loading