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 mysql functional tests #14502

Merged
merged 1 commit into from
Oct 13, 2023
Merged
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
20 changes: 4 additions & 16 deletions .github/workflows/functional_all_db.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ jobs:
- uses: actions/setup-dotnet@v3
with:
dotnet-version: '7.0.x'
- name: Fix nuget restore issue
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a old bug in NuGet service related to SSL certificates. This is not a problem aymore.

run: |
apt update && apt --only-upgrade install ca-certificates -y
- name: Functional Tests
run: |
cd test/OrchardCore.Tests.Functional
Expand Down Expand Up @@ -88,9 +85,6 @@ jobs:
- uses: actions/setup-dotnet@v3
with:
dotnet-version: '7.0.x'
- name: Fix nuget restore issue
run: |
apt update && apt --only-upgrade install ca-certificates -y
- name: Functional Tests
run: |
cd test/OrchardCore.Tests.Functional
Expand All @@ -111,26 +105,23 @@ jobs:
container:
image: cypress/included:9.6.1
services:
mariadb:
image: mariadb:latest
mysql:
Copy link
Member

Choose a reason for hiding this comment

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

Maybe mysql:latest?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I think it's bad to have the version flow automatically as it may break the tests, and we would not test anymore or don't know because it's a new version. It's better to create a PR to change/add a version when it's available and then merge when it works. I will bet you agree since it's the same problem as with using nameof() of column names.

Copy link
Member

Choose a reason for hiding this comment

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

But, if MySQL changed their collation next time, it would be nice to target latest so we are aware of an issue before we bring it to main. Either way, it's not a big deal

Copy link
Member Author

Choose a reason for hiding this comment

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

But, if MySQL changed their collation next time

Yes, we need to be proactive. We are already with all our dependencies, like we are with dotnet.

image: mysql:8
ports:
- 3306
env:
MYSQL_DATABASE: test
MYSQL_ROOT_PASSWORD: test123
options: --health-cmd="mysqladmin ping" --health-interval=5s --health-timeout=2s --health-retries=3
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
env:
OrchardCore__ConnectionString: "server=mariadb;uid=root;pwd=test123;database=test"
OrchardCore__ConnectionString: "server=mysql;uid=root;pwd=test123;database=test"
OrchardCore__DatabaseProvider: "MySql"
steps:
- uses: actions/checkout@v3
# We need to install dotnet in the docker container.
- uses: actions/setup-dotnet@v3
with:
dotnet-version: '7.0.x'
- name: Fix nuget restore issue
run: |
apt update && apt --only-upgrade install ca-certificates -y
- name: Functional Tests
run: |
cd test/OrchardCore.Tests.Functional
Expand Down Expand Up @@ -167,9 +158,6 @@ jobs:
- uses: actions/setup-dotnet@v3
with:
dotnet-version: '7.0.x'
- name: Fix nuget restore issue
run: |
apt update && apt --only-upgrade install ca-certificates -y
- name: Functional Tests
run: |
cd test/OrchardCore.Tests.Functional
Expand Down