-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Added test environment for mssql #4282
Conversation
.drone.yml
Outdated
@@ -360,7 +360,7 @@ services: | |||
image: microsoft/mssql-server-linux:latest | |||
environment: | |||
- ACCEPT_EULA=Y | |||
- SA_PASSWORD=M$wantsaSecurePassword | |||
- SA_PASSWORD=M$wantsaSecurePassword1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need to escape $
character otherwise it tries to connect with password MwantsaSecurePassword1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but maybe not it this file but in app.ini
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing that out, I thinks thats the issue.
Database is not created so gitea is not able to connect: |
@lafriks oh ok. Drone shows a different error message though... But I'll try with your recommondation. |
Now I get that weired error:
Is that an issue with xorm, the test I added, or Gitea? \cc @lunny looks like a xorm issue. |
@kolaente Gitea does not create database so docker service needs to do it. |
@kolaente you have to add some SQL to create the database at first. |
@@ -172,6 +172,20 @@ pipeline: | |||
when: | |||
event: [ push, tag, pull_request ] | |||
|
|||
test-mssql: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: I was being lazy, pay attention to @JonasFranzDEV as his solution is better than mine.
before the test steps (look at group test) you could have a step that creates the DB like:
mssql-create-db:
image: microsoft/mssql-tools
commands:
- sqlcmd -S mssql -U sa -P MwantsaSecurePassword1 -Q "CREATE DATABASE master"
when:
event: [ push, tag, pull_request ]
I haven't tested the above (or indentation) but something like it should work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The better option might be to create the database in code like we do for pgsql and mysql:
https://github.com/go-gitea/gitea/blob/master/integrations/integration_test.go#L110
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've implemented it and the test works now
Signed-off-by: Jonas Franz <[email protected]>
Signed-off-by: Jonas Franz <[email protected]>
Signed-off-by: Jonas Franz <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test environment works but the test fails for other reasons.
@lunny Thanks for the pull request (this is also an issue but not the underlying one) go-xorm/xorm#1173 |
@lunny I'll update it |
Now the build fails, does anyone know if xorm changed significant amounts of code so our code is invalid? |
@kolaente Could you please run a |
Build should work now! |
@kolaente Despite repo_watch using |
@chdxD1 makes sense, I just thought it had beed removed in favour of |
Codecov Report
@@ Coverage Diff @@
## master #4282 +/- ##
==========================================
- Coverage 37.62% 37.59% -0.04%
==========================================
Files 319 319
Lines 46949 46949
==========================================
- Hits 17666 17651 -15
- Misses 26769 26791 +22
+ Partials 2514 2507 -7
Continue to review full report at Codecov.
|
CI passes! 🎉 🎉 🎉 🎉 🎉 🎉 |
@lunny need your approval |
Thanks all of you!!! |
This pr adds a test step in drone to run tests with mssql db.
Even if you dont like Microsoft (like me), IMHO we should at least have tests for mssql dbs or not support it at all.