The README in the test directory doesn't contain any hints on what is needed to run the tests:
- What database users do we need
- What passwords can we use
- Can we override the default config

I think the tests should either honor settings in the environment or have a small (optional, not checked into a version control system) config file.

The test *used to* honor the settings RDBMS_USER and RDBMS_PASSWORD in the environment. They no longer seem to do this. Note that having the password in an environment variable constitutes a security risk. But it is still much better than requiring a hard-coded password in the source.

In addition in Postgres when I do have a database user 'rounduptest' most tests run for me.
But a lot of tests seem to use a username 'rounduptest_schema' with an unknown/undocumented password. I'm currently not able to make these tests run.

Hi Ralf:

I had a few free minutes to respond to this. Hopefully I'll have time
later today to go through your perf changes.

In message <1729520555.54.0.52150611584.issue2551366@roundup.psfhosted.org>,
Ralf Schlatterbeck writes:
>The README in the test directory doesn't contain any hints on what
>is needed to run the tests:
>- What database users do we need
>- What passwords can we use
>- Can we override the default config
>
>I think the tests should either honor settings in the environment or
>have a small (optional, not checked into a version control system) config
>file.

See docs/developer.txt - Testing Notes. It goes over the setup for
developers. live-server tests need the requests package. If you don't
have it the test are (supposed to be) skipped.

Also there are notes in the mysql.txt and postgresql.txt.  Maybe
developer/testing notes in the database docs should be moved to
developers.txt? At least there should be links (currently missing)
from developer.txt to the test sections in the db docs.

>The test *used to* honor the settings RDBMS_USER and RDBMS_PASSWORD
>in the environment. They no longer seem to do this. Note that having
>the password in an environment variable constitutes a security
>risk. But it is still much better than requiring a hard-coded
>password in the source.

Hmm, I never saw/knew that the environment vars were used by the
tests.  I wonder if that changed as part of the patches to use
postgres schemas as well as just a database.

I am a little worried that testing with environment variables that
look like they could be production variables will result in problems
as the tests nuke databases.

>In addition in Postgres when I do have a database user 'rounduptest'
>most tests run for me.
>But a lot of tests seem to use a username 'rounduptest_schema' with
>an unknown/undocumented password. I'm currently not able to make
>these tests run.

See "Running the PostgreSQL unit tests" in ../../doc/postgresql.txt

Are they failing, or are they skipped? I thought I set them up to be
skipped if the schema user/password failed.

References to these docs in a test/README.txt is a good idea.

On Mon, Oct 21, 2024 at 11:06:30AM -0400, John P. Rouillard wrote:

> In message <1729520555.54.0.52150611584.issue2551366@roundup.psfhosted.org>,
> Ralf Schlatterbeck writes:
> >The README in the test directory doesn't contain any hints on what
> >is needed to run the tests:
> >- What database users do we need
> >- What passwords can we use
> >- Can we override the default config
> >
> >I think the tests should either honor settings in the environment or
> >have a small (optional, not checked into a version control system) config
> >file.
> 
> See docs/developer.txt - Testing Notes. It goes over the setup for
> developers. live-server tests need the requests package. If you don't
> have it the test are (supposed to be) skipped.

Thanks, that has some info. But it's missing the database users needed
etc for running the tests. This info probably *should* go into the
developers.txt in the Testing Notes section. And a pointer to this
should be in the README in the test directory.
[ah it's in the database docs]

I do have the requests package. I happen to run an uwsgi daemon on the
hard-coded port 9001. The port should be made configurable in the tests.
But I'll probably move my uwsgi somewhere else for now. Maybe we should
at least skip the tests if the port isn't free.

FAILED test/test_liveserver.py::BaseTestCases::test__generic_item_template_editok - OSError: Ports 9001-9001 are all already in use
[repeated for all tests in test_liveserver]

> Also there are notes in the mysql.txt and postgresql.txt.  Maybe
> developer/testing notes in the database docs should be moved to
> developers.txt? At least there should be links (currently missing)
> from developer.txt to the test sections in the db docs.

Yes links would be fine. And the sections should all be named the same.

> >The test *used to* honor the settings RDBMS_USER and RDBMS_PASSWORD
> >in the environment. They no longer seem to do this. Note that having
> >the password in an environment variable constitutes a security
> >risk. But it is still much better than requiring a hard-coded
> >password in the source.
> 
> Hmm, I never saw/knew that the environment vars were used by the
> tests.  I wonder if that changed as part of the patches to use
> postgres schemas as well as just a database.

Yes, that's possible. It was a long time ago and probably not the best
solution.

> I am a little worried that testing with environment variables that
> look like they could be production variables will result in problems
> as the tests nuke databases.

Good point. My preferred solution would be an optional config-file in
the test directory. With appropriate warnings in the README and the docs
sections.

Is there a technical reason why we need two test users in postgres
currently?

> See "Running the PostgreSQL unit tests" in ../../doc/postgresql.txt

Yes, found it now...

> Are they failing, or are they skipped? I thought I set them up to be
> skipped if the schema user/password failed.

They fail. They are skipped when I don't have any of the postgres users.
I *do* have the first postgres user but not the second one.

Something like:
E       psycopg2.OperationalError: connection to server at "localhost" (127.0.0.1), port 5432 failed: FATAL:  password authentication failed for user "rounduptest_schema"
E       connection to server at "localhost" (127.0.0.1), port 5432 failed: FATAL:  password authentication failed for user "rounduptest_schema"

> References to these docs in a test/README.txt is a good idea.
Yes, and a short summary in the README.

Thanks!
Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com

The following tests fail for me when running with the roundup checkout on an NFS-mounted filesystem:

test/test_anydbm.py::anydbmRestTest::testDispatchGet
test/test_demo.py::TestDemo::testDemoClassic
test/test_demo.py::TestDemo::testDemoJinja
test/test_sqlite.py::sqliteRestTest::testOutputFormat

The first and last fail with
OSError: [Errno 39] Directory not empty: 'db'
or
OSError: [Errno 39] Directory not empty: 'detectors'
respectively

The other two with
E       AssertionError: False is not true : expected db file /home/ralf/checkout/sourceforge/roundup.hg.tip/_test_demo/db/nodes.user or /home/ralf/checkout/sourceforge/roundup.hg.tip/_test_demo/db/nodes.user.dir does not exist
and
E       AssertionError: False is not true : expected db file /home/ralf/checkout/sourceforge/roundup.hg.tip/_test_demo/db/nodes.user does not exist


NFS emulates the Linux filesystem semantics that an open file can be deleted but can still be read/written while it is still open. Since this is not possible for a remote file, NFS keeps a locked .nfsXXX file around which cannot be deleted. This prevents recursive removal of files.

The failing tests probably mean that some file is still open when a recursive removal of files is attempted.

The failing test/test_demo.py tests are not be due to NFS but due to a different dbm backend in use. The file which it asserts should exist is _test_demo/db/nodes.user while on my machine where the test fails this files is named _test_demo/db/nodes.user.db
Note the extension .db

On the machine where this is working I have the debian package
python3-gdbm:amd64

while on the machine where this is failing this is not installed.
So the python builtin dbm support seems to chose different backends depending on what is installed.

I've fixed the failing tests (on the current feature branch)

Except for
test/test_liveserver.py::BaseTestCases::test__generic_item_template_editbad
test/test_liveserver.py::TestFeatureFlagCacheTrackerOff::test__generic_item_template_editbad
test/test_liveserver.py::TestPostgresWsgiServer::test__generic_item_template_editbad

These fail when running the whole test suite but pass when running a single test.
They were marked xfail which I have removed in my current sandbox.

Found the last failing test, too.
The reason this failed when running all tests was that an earlier test monkey-patched some permission checks which subsequently failed a lot of tests later.

In message <1729602020.93.0.576047164055.issue2551366@roundup.psfhosted.org>,
Ralf Schlatterbeck writes:
>I've fixed the failing tests (on the current feature branch)
>
>Except for
>test/test_liveserver.py::BaseTestCases::test__generic_item_template_editbad
>test/test_liveserver.py::TestFeatureFlagCacheTrackerOff::test__generic_item_template_editbad
>test/test_liveserver.py::TestPostgresWsgiServer::test__generic_item_template_editbad
>
>These fail when running the whole test suite but pass when running a single test.
>They were marked xfail which I have removed in my current sandbox.

Thanks. They always worked for me on my development systems but fail
in github CI. Hence the need to xfail them. I was never able to track
down the cause.

What commit fixed this?

Now that all tests pass for me (I'm not yet running any mysql tests): It seems to me we run the same tests twice:

...
test/test_postgresql.py::postgresqlDBTest::testFilteringStringSort PASSED [ 63%]
test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveLinkIssue PASSED [ 63%]
test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveLinkSort PASSED [ 63%]
test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveLinkSortNull PASSED [ 63%]
test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveLinkUser PASSED [ 63%]
test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveLinkUserLimit PASSED [ 63%]
test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveMultilink PASSED [ 63%]
test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveMultilinkSort PASSED [ 63%]
...
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringStringSort PASSED [ 69%]
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringTransitiveLinkIssue PASSED [ 69%]
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringTransitiveLinkSort PASSED [ 69%]
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringTransitiveLinkSortNull PASSED [ 69%]
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringTransitiveLinkUser PASSED [ 69%]
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringTransitiveLinkUserLimit PASSED [ 69%]
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringTransitiveMultilink PASSED [ 69%]
test/test_postgresql.py::postgresqlDBTestSchema::testFilteringTransitiveMultilinkSort PASSED [ 69%]

Do they have any added value when run twice, are there subtle differences in parameters?
They take a lot of time, so we might benefit from removing dupes.

The:

  test/test_postgresql.py::postgresqlDBTest::testFilteringTransitiveMultilinkSort PASSED [ 63%]
  ...
  test/test_postgresql.py::postgresqlDBTestSchema::testFilteringStringSort PASSED [ 69%]

have one significant difference. The first uses a roundup database and creates the tables
etc. in the database. The database user requires createdb and deletedb privs to run.

The second uses a pre-created database and creates schema's (namespaces) inside of the
database. This database user doesn't need create/delete database privs. Hence the new
roundup_schema user. It places the tables inside the schemas rather than directly in the 
database.

(See .github/workflows/ci-test.yml and search for psql for details on the user/db setup.)
I think but am not sure that running a subset of the postgres tests could work
for the schema tests. The differences between these should be in the database user
permissions and database "name".

Maybe we can mark some of the schema tests so a minimal set can run to verify
that we can create a schema and table and test CRUD using the schema.table.
Something like -m "not detail_schema_test" to exclude the marked tests. I'm not
sure marks can be used like this though.

Then we can enable all of the tests for CI to verify that future changes in postgresql, 
psycopg2/psycopg3 etc. don't break.

If this goes forward, it should be a new ticket and not buried in this documentation
ticket.

On Tue, Oct 22, 2024 at 02:20:22PM +0000, John Rouillard wrote:
> >test/test_liveserver.py::BaseTestCases::test__generic_item_template_editbad
> >test/test_liveserver.py::TestFeatureFlagCacheTrackerOff::test__generic_item_template_editbad
> >test/test_liveserver.py::TestPostgresWsgiServer::test__generic_item_template_editbad
> >
> >These fail when running the whole test suite but pass when running a single test.
> >They were marked xfail which I have removed in my current sandbox.
> 
> Thanks. They always worked for me on my development systems but fail
> in github CI. Hence the need to xfail them. I was never able to track
> down the cause.
> 
> What commit fixed this?

changeset:   8129:603aa730b067
Fix failing test due to mokey patching

There is a monkey-patch in that test that wasn't reverted.

It failed only when running all tests, so I triaged that and ran a bunch
of tests plus the one failing until I had it narroved down to a single
test.

KR Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com

Ralf, where is this ticket?

The reference to developer.txt in tests/README.txt has been added. The failing tests are fixed.

Looks like we still need:

 (1) some method for setting postgres and or mysql database users without using
     env vars like RDBMS_USER, RDBMS_PASSWORD.

 skip the tests for postgres schema tests if the postgres rounduptest_schema login is
     unavailable rather than failing. (rounduptest_schema should be generalized to
     ${RDBMS_USER}_schema however RDBMS_USER is defined by (1))

 support/doc for LIVETEST_PORT to change 9001 to a new port number or implement a method
     to find an open port and use that. Possibly include as option with method (1)

 it looks like testing info is fragmented. Maybe we need to create a new
     testing.html and merge all the test setup stuff (including db setup) there? Also
     reference testing.txt from tests/README.txt. Then link to testing to handle:

    * reference postgresql.txt/mysql.txt from developers.txt for testing setup with databases
    * installing.txt needs docs point to developers/mysql/postgres.txt

Anything I am missing?

-- rouilj