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

#118: DB cleanup and IT tests addition to CI #119

Merged
merged 4 commits into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 0 additions & 3 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ jobs:
with:
path: |
${{ github.workspace }}/core/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
${{ github.workspace }}/examples/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
${{ github.workspace }}/slick/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
${{ github.workspace }}/doobie/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
key: ${{ runner.os }}-${{ matrix.scalaShort }}-${{ hashFiles('**/jacoco.xml') }}
Expand Down Expand Up @@ -79,7 +78,6 @@ jobs:
with:
path: |
${{ github.workspace }}/core/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
${{ github.workspace }}/examples/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
${{ github.workspace }}/slick/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
${{ github.workspace }}/doobie/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
key: ${{ runner.os }}-${{ matrix.scalaShort }}-${{ hashFiles('**/jacoco.xml') }}
Expand All @@ -95,7 +93,6 @@ jobs:
${{ github.workspace }}/core/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml,
${{ github.workspace }}/slick/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml,
${{ github.workspace }}/doobie/target/scala-${{ matrix.scalaShort }}/jacoco/report/jacoco.xml
# examples don't need code coverage - at least not now
token: ${{ secrets.GITHUB_TOKEN }}
min-coverage-overall: ${{ matrix.overall }}
min-coverage-changed-files: ${{ matrix.changed }}
Expand Down
61 changes: 61 additions & 0 deletions .github/workflows/integration_tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
#
# Copyright 2022 ABSA Group Limited
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

name: Run Integration Tests

on:
pull_request:
branches:
- '**'
types: [ opened, synchronize, reopened ]

jobs:
run-it:
runs-on: ubuntu-latest
services:
postgres:
image: postgres:15
env:
POSTGRES_PASSWORD: postgres
POSTGRES_DB: movies
options: >-
--health-cmd pg_isready
--health-interval 10s
--health-timeout 5s
--health-retries 5
ports:
- 5432:5432
strategy:
matrix:
scala: [ 2.12.17, 2.13.12 ]
steps:
- name: Checkout code
uses: actions/checkout@v2
- uses: coursier/cache-action@v5

- name: Setup Scala
uses: olafurpg/setup-scala@v14
with:
java-version: "[email protected]"

- name: Prepare testing database
run: sbt flywayMigrate

- name: Build and run IT tests for Doobie
run: sbt "project faDBDoobie" ++${{matrix.scala}} it:test

- name: Build and run IT tests for Slick
run: sbt "project faDBSlick" ++${{matrix.scala}} it:test
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,11 @@ Code coverage will be generated on path:

### Integration tests

There are now integration tests as part of the project (at the time of writing they are in the _Slick_ module).
There are now integration tests as part of the project (at the time of writing they are in
the _Slick_ and _Doobie_ modules).

For the tests to work properly a running Postgres instance is needed. And then the following setup:
* execute (content of) all `*.sql` files within `it/resources/sql/` folder within a posgres query tool
* modify `it/resources/application.conf` to point to the database used in the previous point
For the tests to work properly a running Postgres instance is needed as well as all DB objects must be placed on the DB.
We automated this process, see `database/README.md` for more details.

How to execute the tests:

Expand All @@ -185,4 +185,4 @@ sbt it:test

## How to Release

Please see [this file](RELEASE.md) for more details.
Please see [this file](RELEASE.md) for more details.
24 changes: 9 additions & 15 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,16 @@ lazy val commonJacocoReportSettings: JacocoReportSettings = JacocoReportSettings
lazy val commonJacocoExcludes: Seq[String] = Seq(
)

enablePlugins(FlywayPlugin)
flywayUrl := FlywayConfiguration.flywayUrl
flywayUser := FlywayConfiguration.flywayUser
flywayPassword := FlywayConfiguration.flywayPassword
flywayLocations := FlywayConfiguration.flywayLocations
flywaySqlMigrationSuffixes := FlywayConfiguration.flywaySqlMigrationSuffixes
libraryDependencies ++= flywayDependencies
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it adds to the global dependencies, doesn't it? I think flyway dependencies shouldn't be a generic dependency.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is what @salamonpavel was dealing with in the past and there was no way how to circumvent this (I know :( ) by extracting / moving elsewhere. Anyway, it's part of the build, i.e. general dependency: https://github.com/AbsaOSS/atum-service/blob/master/build.sbt#L53-L59

(remember, Flyway is temporary anyway)

Copy link
Contributor

@Zejnilovic Zejnilovic Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @benedeki

lazy val modulNeedingFlyaway = project
  .enablePlugins(FlywayPlugin)
  .settings(
    flywayUrl := FlywayConfiguration.flywayUrl
    flywayUser := FlywayConfiguration.flywayUser
    flywayPassword := FlywayConfiguration.flywayPassword
    flywayLocations := FlywayConfiguration.flywayLocations
    flywaySqlMigrationSuffixes := FlywayConfiguration.flywaySqlMigrationSuffixes
    libraryDependencies ++= flywayDependencies
  )

and then run it like sbt modulNeedingFlyaway/flywayMigrate

Are we saying there is an issue with this @lsulak ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we redefine the flywayMigrate to behave like sbt modulNeedingFlyaway/flywayMigrate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like this addCommandAlias("flywayMigrate", "modulNeedingFlyaway/flywayMigrate")?

Copy link
Collaborator Author

@lsulak lsulak Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me try, thanks @Zejnilovic. To be fair I haven't been playing with this one as my primary objective here was to actually have an IT suite embedded in our CI & I borrowed what was done in Atum Service - but I agree that we can improve this mechanism in build.sbt

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why it got approved, not blocking the whole thing 😉

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested, it works very nicely, thank you Sasa! #124


benedeki marked this conversation as resolved.
Show resolved Hide resolved
lazy val parent = (project in file("."))
.aggregate(faDbCore, faDBSlick, faDBDoobie, faDBExamples)
.aggregate(faDbCore, faDBSlick, faDBDoobie)
.settings(
name := "root",
libraryDependencies ++= rootDependencies(scalaVersion.value),
Expand Down Expand Up @@ -102,18 +110,4 @@ lazy val faDBDoobie = (project in file("doobie"))
jacocoExcludes := commonJacocoExcludes
)

lazy val faDBExamples = (project in file("examples"))
.configs(IntegrationTest)
.settings(
name := "examples",
libraryDependencies ++= examplesDependencies(scalaVersion.value),
Test / parallelExecution := false,
(Compile / compile) := ((Compile / compile) dependsOn printScalaVersion).value, // printScalaVersion is run with compile
publish / skip := true
).dependsOn(faDbCore, faDBSlick)
.settings(
jacocoReportSettings := commonJacocoReportSettings.withTitle(s"fa-db:examples Jacoco Report - scala:${scalaVersion.value}"),
jacocoExcludes := commonJacocoExcludes
)

sonatypeProfileName := "za.co.absa"
41 changes: 41 additions & 0 deletions demo_database/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## About

This module implements a simple database with many types of objects (tables, functions, data insertions, and more)
that will be used in integration tests in these modules:
* `doobie/src/it/`
* `slick/src/it/`

## Deployment

How to set up database for local testing

### Using Docker

```zsh
# start up postgres docker container (optional; instead you can create movies on your local postgres instance)
docker run --name=movies -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=movies -p 5432:5432 -d postgres:16

# migrate scripts
sbt flywayMigrate

# kill & remove docker container (optional; only if using dockerized postgres instance)
docker kill aul_db
docker rm aul_db
```

### Using local postgres instance

```zsh
# migrate scripts
sbt flywayMigrate
```

In case some structures are already present in the database, you can use
```zsh
sbt flywayClean
```
to remove them or
```zsh
sbt flywayBaseline
```
to set the current state as the baseline.
19 changes: 19 additions & 0 deletions demo_database/src/main/postgres/00_databases.ddl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright 2021 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE DATABASE movies
WITH
ENCODING = 'UTF8'
CONNECTION LIMIT = -1;
18 changes: 18 additions & 0 deletions demo_database/src/main/postgres/V1.1.1__01_add_extensions.ddl
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright 2021 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE EXTENSION IF NOT EXISTS hstore;
CREATE EXTENSION IF NOT EXISTS ltree;
CREATE EXTENSION IF NOT EXISTS "uuid-ossp"; -- for function `uuid_generate_v4`
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE OR REPLACE FUNCTION integration.get_all_date_time_types(p_id INT)
RETURNS TABLE(
offset_date_time TIMESTAMPTZ,
instant TIMESTAMPTZ,
zoned_date_time TIMESTAMPTZ,
local_date_time TIMESTAMP,
local_date DATE,
local_time TIME,
sql_date DATE,
sql_time TIME,
sql_timestamp TIMESTAMP,
util_date TIMESTAMP
) AS $$
BEGIN
RETURN QUERY SELECT
T.offset_date_time,
T.instant,
T.zoned_date_time,
T.local_date_time,
T.local_date,
T.local_time,
T.sql_date,
T.sql_time,
T.sql_timestamp,
T.util_date
FROM integration.date_time_types T limit p_id;
END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright 2022 ABSA Group Limited
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

CREATE OR REPLACE FUNCTION integration.insert_dates_times(
IN p_offset_date_time TIMESTAMPTZ,
IN p_instant TIMESTAMPTZ,
IN p_zoned_date_time TIMESTAMPTZ,
IN p_local_date_time TIMESTAMP,
IN p_local_date DATE,
IN p_local_time TIME,
IN p_sql_date DATE,
IN p_sql_time TIME,
IN p_sql_timestamp TIMESTAMP,
IN p_util_date DATE,
OUT status INTEGER,
OUT status_text TEXT,
OUT o_id INTEGER
) RETURNS record AS $$
BEGIN
INSERT INTO integration.date_time_types (
offset_date_time,
instant,
zoned_date_time,
local_date_time,
local_date,
local_time,
sql_date,
sql_time,
sql_timestamp,
util_date
) VALUES (
p_offset_date_time,
p_instant,
p_zoned_date_time,
p_local_date_time,
p_local_date,
p_local_time,
p_sql_date,
p_sql_time,
p_sql_timestamp,
p_util_date
) RETURNING id INTO o_id;

status := 11;
status_text := 'OK';

RETURN;
END;
$$ LANGUAGE plpgsql;
Original file line number Diff line number Diff line change
Expand Up @@ -14,51 +14,7 @@
* limitations under the License.
*/

CREATE TABLE runs.other_types (
id INT PRIMARY KEY,
ltree_col LTREE,
inet_col INET,
macaddr_col MACADDR,
hstore_col HSTORE,
cidr_col CIDR,
json_col JSON,
jsonb_col JSONB,
uuid_col UUID,
array_col INT[]
);

INSERT INTO runs.other_types VALUES (
1,
'Top.Science.Astronomy',
'192.168.1.1',
'08:00:2b:01:02:03',
'key=>value',
'192.168.1/24',
'{"key": "value"}',
'{"key": "value"}',
uuid_generate_v4(),
ARRAY[1,2,3]
);

CREATE OR REPLACE FUNCTION runs.read_other_types(p_id INT)
RETURNS TABLE(
id INT,
ltree_col LTREE,
inet_col INET,
macaddr_col MACADDR,
hstore_col HSTORE,
cidr_col CIDR,
json_col JSON,
jsonb_col JSONB,
uuid_col UUID,
array_col INT[]
) AS $$
BEGIN
RETURN QUERY SELECT * FROM runs.other_types T WHERE T.id = p_id;
END;
$$ LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION runs.insert_other_types(
CREATE OR REPLACE FUNCTION integration.insert_other_types(
p_id INT,
p_ltree_col LTREE,
p_inet_col INET,
Expand All @@ -76,7 +32,7 @@ CREATE OR REPLACE FUNCTION runs.insert_other_types(
) AS $$
BEGIN
BEGIN
INSERT INTO runs.other_types VALUES (
INSERT INTO integration.other_types VALUES (
p_id,
p_ltree_col,
p_inet_col,
Expand Down
Loading
Loading