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

Convert functional test SQL to YAML #1506

Merged
merged 4 commits into from Mar 10, 2020

Conversation

@aschempp
Copy link
Contributor

aschempp commented Mar 9, 2020

requires contao/test-case#5

@aschempp aschempp added the feature label Mar 9, 2020
@aschempp aschempp added this to the 4.10 milestone Mar 9, 2020
@aschempp aschempp requested review from leofeyer and contao/reviewers Mar 9, 2020
@aschempp aschempp self-assigned this Mar 9, 2020
Copy link
Member

Toflar left a comment

I wonder why we need news archive data for the existing tests but maybe that can be removed.
But not strictly related to this PR as that seems to be the migration only for now, so I'm approving.

@aschempp aschempp force-pushed the aschempp:feature/functional-tests-yml branch from 10b2da4 to 8b88409 Mar 9, 2020
Copy link
Member

leofeyer left a comment

You should also adjust the .github/workflows/ci.yml file, which still tries to import the contao_test.sql dump. Also, we might need to raise the Composer requirement for contao/test-case to fix the prefer-lowest tests.

@aschempp

This comment has been minimized.

Copy link
Contributor Author

aschempp commented Mar 9, 2020

You should also adjust the .github/workflows/ci.yml file, which still tries to import the contao_test.sql dump.

I can't find that, and it wouldn't make sense anyway? The SQL dump is/was loaded by the test case?

Also, we might need to raise the Composer requirement for contao/test-case to fix the prefer-lowest tests.

Absolutely, but the new branch is not tagged. It should probably be v5 as it's a BC-break from the old implementation?

@leofeyer

This comment has been minimized.

Copy link
Member

leofeyer commented Mar 9, 2020

I can't find that, and it wouldn't make sense anyway? The SQL dump is/was loaded by the test case?

True, we had already changed that.

Also, we might need to raise the Composer requirement for contao/test-case to fix the prefer-lowest tests.

Absolutely, but the new branch is not tagged. It should probably be v5 as it's a BC-break from the old implementation?

It should be enough to require "contao/test-case": "^4.1" in our composer.json.

@aschempp

This comment has been minimized.

Copy link
Contributor Author

aschempp commented Mar 9, 2020

It should be enough to require "contao/test-case": "^4.1" in our composer.json.

I don't think so? The base 4.1 version (from last week) does not contain my "bug fix"?

@leofeyer

This comment has been minimized.

Copy link
Member

leofeyer commented Mar 9, 2020

@leofeyer leofeyer merged commit 38f3275 into contao:master Mar 10, 2020
8 checks passed
8 checks passed
Coverage
Details
Coding Style
Details
PHP 7.2
Details
PHP 7.3
Details
PHP 7.4
Details
Prefer Lowest
Details
Bundles
Details
Windows
Details
@aschempp aschempp deleted the aschempp:feature/functional-tests-yml branch Mar 10, 2020
@aschempp aschempp mentioned this pull request Mar 10, 2020
15 of 20 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.