Do not hard-code PostgreSQL credentials in tester file

[strk]

This allows running tests against non-default installs

NOTE: I suppose the change in .drone.yml will require signing it
again, which I cannot do. Can you help @lunny $bkcsoft $appleboy ?

[lafriks]

I think Makefile also needs changes so that running make test-pgsql etc would also have defaults

[lafriks]

and then you would probably not need .drone.yml changes

[lafriks]

Adding these lines somewhere in the beginning of Makefile:

PGUSER ?= postgres
PGPASSWORD ?= postgres
PGHOST ?= 127.0.0.1
PGPORT ?= 5432

and using your pgsql.ini file allows me to run PGUSER=testgitea make test-pgsql successfully

[lafriks]

Do not change .drone.yml file but make changes only to Makefile as I suggested in comments

[strk]

the reason I had to make these changes was to be able to run make test-pgsql on my system, and I was able due to my libpq defaults and/or libpq supported env variables. If I hard-code those values in the Makefile the purpose of this change would be defeated. My point is that it should be configurable how to connect, and it is already, via libpq supported env variables. Let's not override those with hard-coded values.

Note I already changed .drone.yml in this PR. And I'm happy to see there was no need to update the signature file (drone/pr build succeeds)

[strk]

I dunno why those many changes were added to the .ini file though, I only removed 3 lines, must be something related to .editorconfig, should I revert-clean ?

[strk]

force-pushed after cleaning up the changes in .ini file

[daviian]

The changes are created by make test-pgsql

[strk]

oh, that's an issue that should also be fixed (maybe in a different PR).
Source files should not be changed by test targets, IMHO

[daviian]

I automatically do a git reset integrations/pgsql.ini so it wasn't a problem for me til now ;-)

[lafriks]

@strk if you have these variables set makefile wont change them but just reuse yours. ?= sets just defaults if you don't have environment variables so it will work for everyone.

[lafriks]

Looks like you also need to change:

test-pgsql: integrations.test
	GITEA_ROOT=${CURDIR} GITEA_CONF=integrations/pgsql.ini ./integrations.test

to:

test-pgsql: integrations.test
	PGHOST=${PGHOST} PGPORT=${PGPORT} PGUSER=${PGUSER} PGPASSWORD=${PGPASSWORD} GITEA_ROOT=${CURDIR} GITEA_CONF=integrations/pgsql.ini ./integrations.test

[strk]

Using `?=` would still force an env variable when all you want is the libpq default (ie: compile-time default to using custom TCP port, for instance) What's wrong with relying on libpq default ?

[lafriks]

ok, you convinced me through I will still need to set env variables to run tests 👍

[lafriks]

@go-gitea/owners does .drone.yml needs to be signed again?

[lafriks]

otherwise LGTM

[strk]

@daviian for the record #2346 fixes the override of source file (if you want to test)

[lafriks]

As #2484 is merged are these changes still needed?

[strk]

Well I like it more to be able to use my already-existing environment rather than (as done by #2484) enforce the use of custom env variables...

I've rebased to master and fixed conflicts.

[strk]

The thing is that enforcing a setting for PG host/user/port prevents using libpq defaults. I like the idea of using what already exists (defaults)

[strk]

The integration test actually still fails for me :(

Unsupported RDBMS for integration tests
Makefile:181: recipe for target 'test-pgsql' failed

Ideas ?

[lafriks]

pghost must be pgsql not 127.0.0.1 in drone

[lafriks]

I don't think it's correct to use user defaults for integration tests as at least for me PG defaults are used for PostgreSQL admin user (username&host) so I don't have to use that for psql command. I would not like to use them for integration tests. Current approach is more configurable for everyone as you can set specific environment variables just for gitea and easily run just make test-pgsql

[techknowlogick]

Closing as outdated. Please re-open as needed.