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

EZP-31246: Refactored Test setup to stop relying on Database Handler #14

Merged

Conversation

alongosz
Copy link
Member

@alongosz alongosz commented Mar 27, 2020

Question Answer
JIRA issue EZP-31246 blocking EZP-30921
Type improvement
Target version ezplatform-kernel:1.0 (internal package test setup)
BC breaks no
Tests pass yes
Doc needed no

TL;DR; Key points:

  1. Yaml test data: ab1c973 and c2ecf1a
  2. FixtureImporter: fcaa722
  3. Usage of FixtureImporter: a9cfb3a and 1dae04a

Summary of changes

This PR drops most of[1] eZc Database Handler usages from our tests (both integration and unit).
You can review it commit by commit - I tried to keep them as separate as possible, though maintaining an order during reviewing is probably recommended, as some changes rely on previous changes.

The most interesting aspect though, is not the refactoring of query building, but concepts I introduced in order to instantiate test setup. It heavily relied on Database Handler, so had to be changed anyway.

The initial data fixtures were stored in eZ/Publish/Core/Repository/Tests/Service/Integration/Legacy/_fixtures/test_data.php.
The file was big, triggered unnecessary static analysis, and overall was difficult to maintain.

I decided to go with Yaml for Kernel 1.0: eZ/Publish/API/Repository/Tests/_fixtures/Legacy/data/test_data.yaml.
It feels more light for IDE in terms of both viewing and updating. It's obviously still too big, but one thing at a time.
Added via ab1c973 and updated via c2ecf1a.

As an intermediate step, I've upgraded SetupFactory to use that new data file and import it using Doctrine QueryBuilder (7908cea).
This led to another key point, which is moving this responsibility to a set of separate classes. Hence, I've introduced via fcaa722 FixtureImporter with in-memory caching layer and Fixture instances for PHP files (for some cases) and Yaml files (the only one now - test_data.yaml). It greatly improved readability of both Setup Factory (a9cfb3a) and unit tests base TestCase (1dae04a).

Less important thing, but useful in the long run: \eZ\Publish\Core\Persistence\Legacy\Tests\TestCase::getDatabaseConnection now fails a test case on connection error instead of re-throwing. Connection error usually does not require analyzing stack trace anyway. This change will make our test cases fetching connection not to report unhandled exception (dbeb8f7).

You might also wonder why I've dropped some tests via 25dccae (eZ\Publish\Core\Persistence\Doctrine\Tests). Those are for Database Handler being dropped, this is why they're gone.

Other commits are strictly related to refactoring.


[1] There's still \eZ\Publish\Core\Persistence\Legacy\Tests\TestCase::getDatabaseHandler which I cannot drop until #11 (or PRs to be split from that) are completed and merged.

Checklist:

  • PR description is updated.
  • Tests are passing.
  • Added code follows Coding Standards (use $ composer fix-cs).
  • PR is ready for a review.

Changed Core\Persistence TestCase::getDatabaseConnection to fail
instead of throwing exception (makes no sense for test case
to report it thrown and then report it for every test using that
method as thrown as well).
The new data fixtures file is located at: eZ/Publish/API/Repository/Tests/_fixtures/Legacy/data/test_data.yaml
@alongosz alongosz force-pushed the ezp-31246-drop-dbhandler-from-tests branch from 130c80e to 8fc23a6 Compare March 27, 2020 13:50
alongosz added a commit that referenced this pull request Mar 28, 2020
…14)

Squashed commit of the following:

commit 8fc23a6
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 27 12:40:27 2020 +0100

    Dropped test_data.php in favor of test_data.yaml

    The new data fixtures file is located at: eZ/Publish/API/Repository/Tests/_fixtures/Legacy/data/test_data.yaml

commit 4493e6d
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 27 12:39:28 2020 +0100

    Refactored remaining unit tests to stop relying on test_data.php

commit 16e38ec
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 13 12:04:39 2020 +0100

    [Unit Tests] Replaced DatabaseHandler usages with Doctrine\DBAL

commit 7768654
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Sat Mar 7 17:36:35 2020 +0100

    [Tests] Refactored unit tests to rely on Doctrine QueryBuilder

commit 06f5a7f
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Wed Dec 18 16:14:05 2019 +0100

    Refactored TestCase::assertQueryResult accept QueryBuilder

commit dbeb8f7
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Thu Dec 19 16:47:27 2019 +0100

    Changed Core\Persistence TestCase::getDatabaseConnection to fail on err.

    Changed Core\Persistence TestCase::getDatabaseConnection to fail
    instead of throwing exception (makes no sense for test case
    to report it thrown and then report it for every test using that
    method as thrown as well).

commit 1dae04a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Wed Dec 18 16:08:58 2019 +0100

    Refactored unit tests setups to use FixtureImporter

commit a9cfb3a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Dec 17 17:12:44 2019 +0100

    Refactored Legacy Storage Setup Factory to use FixtureImporter

commit fcaa722
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Dec 17 16:47:12 2019 +0100

    Implemented database fixtures importer

commit 7908cea
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Mon Dec 16 17:02:28 2019 +0100

    Refactored integration tests data import to stop relying on DB Handler

commit c2ecf1a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Sun Mar 15 01:20:34 2020 +0100

    Updated test data LSE FullText index

commit ab1c973
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Mon Dec 16 16:24:06 2019 +0100

    Changed format of integration tests data fixtures into Yaml

commit 25dccae
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Oct 29 16:01:46 2019 +0100

    [Tests] Dropped eZ\Publish\Core\Persistence\Doctrine\Tests
alongosz added a commit that referenced this pull request Mar 28, 2020
…14)

Squashed commit of the following:

commit 8fc23a6
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 27 12:40:27 2020 +0100

    Dropped test_data.php in favor of test_data.yaml

    The new data fixtures file is located at: eZ/Publish/API/Repository/Tests/_fixtures/Legacy/data/test_data.yaml

commit 4493e6d
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 27 12:39:28 2020 +0100

    Refactored remaining unit tests to stop relying on test_data.php

commit 16e38ec
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 13 12:04:39 2020 +0100

    [Unit Tests] Replaced DatabaseHandler usages with Doctrine\DBAL

commit 7768654
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Sat Mar 7 17:36:35 2020 +0100

    [Tests] Refactored unit tests to rely on Doctrine QueryBuilder

commit 06f5a7f
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Wed Dec 18 16:14:05 2019 +0100

    Refactored TestCase::assertQueryResult accept QueryBuilder

commit dbeb8f7
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Thu Dec 19 16:47:27 2019 +0100

    Changed Core\Persistence TestCase::getDatabaseConnection to fail on err.

    Changed Core\Persistence TestCase::getDatabaseConnection to fail
    instead of throwing exception (makes no sense for test case
    to report it thrown and then report it for every test using that
    method as thrown as well).

commit 1dae04a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Wed Dec 18 16:08:58 2019 +0100

    Refactored unit tests setups to use FixtureImporter

commit a9cfb3a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Dec 17 17:12:44 2019 +0100

    Refactored Legacy Storage Setup Factory to use FixtureImporter

commit fcaa722
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Dec 17 16:47:12 2019 +0100

    Implemented database fixtures importer

commit 7908cea
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Mon Dec 16 17:02:28 2019 +0100

    Refactored integration tests data import to stop relying on DB Handler

commit c2ecf1a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Sun Mar 15 01:20:34 2020 +0100

    Updated test data LSE FullText index

commit ab1c973
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Mon Dec 16 16:24:06 2019 +0100

    Changed format of integration tests data fixtures into Yaml

commit 25dccae
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Oct 29 16:01:46 2019 +0100

    [Tests] Dropped eZ\Publish\Core\Persistence\Doctrine\Tests
alongosz added a commit that referenced this pull request Mar 29, 2020
…14)

Squashed commit of the following:

commit 8fc23a6
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 27 12:40:27 2020 +0100

    Dropped test_data.php in favor of test_data.yaml

    The new data fixtures file is located at: eZ/Publish/API/Repository/Tests/_fixtures/Legacy/data/test_data.yaml

commit 4493e6d
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 27 12:39:28 2020 +0100

    Refactored remaining unit tests to stop relying on test_data.php

commit 16e38ec
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Fri Mar 13 12:04:39 2020 +0100

    [Unit Tests] Replaced DatabaseHandler usages with Doctrine\DBAL

commit 7768654
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Sat Mar 7 17:36:35 2020 +0100

    [Tests] Refactored unit tests to rely on Doctrine QueryBuilder

commit 06f5a7f
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Wed Dec 18 16:14:05 2019 +0100

    Refactored TestCase::assertQueryResult accept QueryBuilder

commit dbeb8f7
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Thu Dec 19 16:47:27 2019 +0100

    Changed Core\Persistence TestCase::getDatabaseConnection to fail on err.

    Changed Core\Persistence TestCase::getDatabaseConnection to fail
    instead of throwing exception (makes no sense for test case
    to report it thrown and then report it for every test using that
    method as thrown as well).

commit 1dae04a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Wed Dec 18 16:08:58 2019 +0100

    Refactored unit tests setups to use FixtureImporter

commit a9cfb3a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Dec 17 17:12:44 2019 +0100

    Refactored Legacy Storage Setup Factory to use FixtureImporter

commit fcaa722
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Dec 17 16:47:12 2019 +0100

    Implemented database fixtures importer

commit 7908cea
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Mon Dec 16 17:02:28 2019 +0100

    Refactored integration tests data import to stop relying on DB Handler

commit c2ecf1a
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Sun Mar 15 01:20:34 2020 +0100

    Updated test data LSE FullText index

commit ab1c973
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Mon Dec 16 16:24:06 2019 +0100

    Changed format of integration tests data fixtures into Yaml

commit 25dccae
Author: Andrew Longosz <alongosz@users.noreply.github.com>
Date:   Tue Oct 29 16:01:46 2019 +0100

    [Tests] Dropped eZ\Publish\Core\Persistence\Doctrine\Tests
@alongosz alongosz requested a review from adamwojs March 30, 2020 11:39
@alongosz alongosz merged commit 6da412b into ezsystems:master Mar 31, 2020
@alongosz alongosz deleted the ezp-31246-drop-dbhandler-from-tests branch March 31, 2020 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Changes not fixing or changing behavior Ready for review
Development

Successfully merging this pull request may close these issues.

3 participants