When testing patch for issue2550636, it turns out that none of
the tests in test_indexer.py would run. I don't know what is wrong
but I worked around it by commenting out the import and classes
for the backends that I did not have installed.

I poked at it trying to fix/diagnose the decorators used to prevent
the tests for the missing components from running, but my python-fu
isn't up to it.

My environment was: linux mint 17.3

> ./run_tests.py --debug -r a --verbose --capture=sys test/test_indexer.py
writing pytestdebug information to
/home/rouilj/develop/roundup.dev/roundup.hg/pytestdebug.log
============================= test session starts
==============================
platform linux2 -- Python 2.7.6, pytest-2.8.4, py-1.4.30, pluggy-0.3.1
-- /usr/bin/python
using: pytest-2.8.4 pylib-1.4.30
cachedir: .cache
rootdir: /home/rouilj/develop/roundup.dev/roundup.hg, inifile: 
collected 0 items / 1 skipped 

=========================== short test summary info
============================
SKIP [1]
/home/rouilj/develop/roundup.dev/roundup.hg/test/test_mysql.py:53:
Skipping MySQL tests: (2002, "Can't connect to local MySQL server
through socket '/var/run/mysqld/mysqld.sock' (2)")
========================== 1 skipped in 3.36 seconds
===========================
wrote pytestdebug information to
/home/rouilj/develop/roundup.dev/roundup.hg/pytestdebug.log

Yes, I confirm this bug. I believe this is related to an open pytest issue:

  https://github.com/pytest-dev/pytest/issues/568

I thought I had worked-around the issue and tested it, but apparently not.

The pytest issue contains a reference to the following work-around:

 
https://github.com/Parsely/pykafka/blob/e18bdeb3586fab21493b755d564d6b4aa8ec7ef2/tests/pykafka/__init__.py#L4

I've had a quick look and it seems to work, so I'll look into preparing
a patch for the roundup tests to use until the bug is fixed in pytest.

On closer inspection the solution used by pykafka may not work because
of roundup's use of multiple inheritance for is DB tests.

I also noticed that the the tests are currently using 'pytest.skip'
instead of 'pytest.marker.skip', which probably doesn't help.

I'll keep doing a bit more work to see if I can find a suitable workaround.

Work-around for pytest marker bug related to skipping tests fixed in commit:

  https://sourceforge.net/p/roundup/code/ci/37d1e24fb9

run_tests.py updated to latest pytest version to fix bug with
'pytest.marker.skip' not correctly skipping tests in commit:

  https://sourceforge.net/p/roundup/code/ci/07168ae926

I believe this issue can be closed for now, unless you are still
experiencing problems John?

Well it's better but postgres tests still being run.

I just hg pulled and hg updated. hg diff reports no differences.

Running:

 ./run_test --verbose test/test_indexer.py gives me:

test/test_indexer.py::XapianIndexerTest::test_stopwords PASSED
test/test_indexer.py::XapianIndexerTest::test_wordsplitting PASSED
test/test_indexer.py::postgresqlIndexerTest::test_basics FAILED
test/test_indexer.py::postgresqlIndexerTest::test_casesensitity FAILED
test/test_indexer.py::postgresqlIndexerTest::test_change FAILED
test/test_indexer.py::postgresqlIndexerTest::test_clear FAILED
test/test_indexer.py::postgresqlIndexerTest::test_extremewords FAILED
test/test_indexer.py::postgresqlIndexerTest::test_manyresults FAILED
test/test_indexer.py::postgresqlIndexerTest::test_stopwords FAILED
test/test_indexer.py::postgresqlIndexerTest::test_wordsplitting FAILED
test/test_indexer.py::mysqlIndexerTest::test_basics SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_casesensitity SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_change SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_clear SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_extremewords SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_manyresults SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_stopwords SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_wordsplitting SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_basics SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_casesensitity SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_change SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_clear SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_extremewords SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_manyresults SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_stopwords SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_wordsplitting SKIPPED

sqlite shouldn't be skipped. Postgres should be skipped not fail.

All the postgres errors are of the form:

>           raise hyperdb.DatabaseError(message)
E           DatabaseError: could not connect to server: Connection refused
E           	Is the server running on host "localhost" (127.0.0.1) and
accepting
E           	TCP/IP connections on port 5432?

roundup/backends/back_postgresql.py:91: DatabaseError

If I move the definiton do the sqlite test before postgres and mysql,
they all pass. So it looks like the @skip_mysql decorator is
applying to the

class sqliteIndexerTest(sqliteOpener, RDBMSIndexerTest, IndexerTest):
    pass

unless I move this class definition above postgres.

If I reorder some more and move mysql above postgres I get:

[...]
test/test_indexer.py::sqliteIndexerTest::test_stopwords PASSED
test/test_indexer.py::sqliteIndexerTest::test_wordsplitting PASSED
test/test_indexer.py::mysqlIndexerTest::test_basics SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_casesensitity SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_change SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_clear SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_extremewords SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_manyresults SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_stopwords SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_wordsplitting SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_basics SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_casesensitity SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_change SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_clear SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_extremewords SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_manyresults SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_stopwords SKIPPED
test/test_indexer.py::postgresqlIndexerTest::test_wordsplitting SKIPPED

So not sure what to do here. Ideas?

-- rouilj

On 28/06/16 08:26, John Rouillard wrote:
> Well it's better but postgres tests still being run.

Check to see if you have the 'psycopg' python package installed. The 
tests rely on using 'roundup.backend.have_backend()'[1] to determine if 
the backend is available and will attempt to run the tests if it returns 
True. The 'have_backend()' essentially returns True for postgresql if 
the 'psycopg' package is installed.

> sqlite shouldn't be skipped. Postgres should be skipped not fail.
>
> All the postgres errors are of the form:
>
>>           raise hyperdb.DatabaseError(message)
> E           DatabaseError: could not connect to server: Connection refused
> E           	Is the server running on host "localhost" (127.0.0.1) and
> accepting
> E           	TCP/IP connections on port 5432?
>
> roundup/backends/back_postgresql.py:91: DatabaseError
>
> If I move the definiton do the sqlite test before postgres and mysql,
> they all pass. So it looks like the @skip_mysql decorator is
> applying to the
>
> class sqliteIndexerTest(sqliteOpener, RDBMSIndexerTest, IndexerTest):
>     pass
>
> unless I move this class definition above postgres.

It seems as though 'pytest.mark.skip()' bug[2] taints any test that 
shares a parent test class, not just the other tests that are marked to 
be skipped by pytest - which is what I thought the problem was.

I'll do a bit more looking to see if an improved work-around can be found.

[1] 
https://sourceforge.net/p/roundup/code/ci/37d1e24fb9/tree/roundup/backends/__init__.py#l46

[2] https://github.com/pytest-dev/pytest/issues/568

OK, another attempt at fixing this has been committed in:

  https://sourceforge.net/p/roundup/code/ci/43a1f7fe39/

Hi John:

In message <e741293b-947c-0f5e-6774-e0df1ae25452@jerrykan.com>,
John Kristensen writes:
>John Kristensen added the comment:
>On 28/06/16 08:26, John Rouillard wrote:
>> Well it's better but postgres tests still being run.
>
>Check to see if you have the 'psycopg' python package installed. The 
>tests rely on using 'roundup.backend.have_backend()'[1] to determine if 
>the backend is available and will attempt to run the tests if it returns 
>True. The 'have_backend()' essentially returns True for postgresql if 
>the 'psycopg' package is installed.

Hmm, mysql IIRC tries to conenct to the db as part of have_backend()
or some wrapper around have_backend() in the tests. If it can't
connect it skips.

I'll see if I can figure out how mysql handles this as I have both
libraries installed just not the databases.
>> [elided tests being skipped that shouldn't be.]
>It seems as though 'pytest.mark.skip()' bug[2] taints any test that 
>shares a parent test class, not just the other tests that are marked to 
>be skipped by pytest - which is what I thought the problem was.
>
>I'll do a bit more looking to see if an improved work-around can be found.

Thanks for trying to track this down.

Hi Jerry:

Your latest patch sees to have done much of the trick.
...
test/test_indexer.py::XapianIndexerTest::test_manyresults PASSED
test/test_indexer.py::XapianIndexerTest::test_stopwords PASSED
test/test_indexer.py::XapianIndexerTest::test_wordsplitting PASSED
test/test_indexer.py::postgresqlIndexerTest::test_basics FAILED
test/test_indexer.py::postgresqlIndexerTest::test_casesensitity FAILED
test/test_indexer.py::postgresqlIndexerTest::test_change FAILED
test/test_indexer.py::postgresqlIndexerTest::test_clear FAILED
test/test_indexer.py::postgresqlIndexerTest::test_extremewords FAILED
test/test_indexer.py::postgresqlIndexerTest::test_manyresults FAILED
test/test_indexer.py::postgresqlIndexerTest::test_stopwords FAILED
test/test_indexer.py::postgresqlIndexerTest::test_wordsplitting FAILED
test/test_indexer.py::mysqlIndexerTest::test_basics SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_casesensitity SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_change SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_clear SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_extremewords SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_manyresults SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_stopwords SKIPPED
test/test_indexer.py::mysqlIndexerTest::test_wordsplitting SKIPPED
test/test_indexer.py::sqliteIndexerTest::test_basics PASSED
test/test_indexer.py::sqliteIndexerTest::test_casesensitity PASSED
test/test_indexer.py::sqliteIndexerTest::test_change PASSED
...

I'll need to figure out how mysql gets skipped but postgres doesn't
as import MySQLdb works fine and the backend check seems to pass
but mysql tests are still properly skipped.

Thanks.

Sigh, John looks like your fix broke something else.
I just ran a full test before checkin in some additional tests I
developed and I see:

test/test_mysql.py::mysqlSchemaTest::test_removeClassKey <-
test/db_test_base.py SKIPPED
test/test_mysql.py::mysqlSchemaTest::test_removeMultilink <-
test/db_test_base.py SKIPPED
test/test_mysql.py::mysqlSchemaTest::test_reservedProperties <-
test/db_test_base.py SKIPPED
test/test_mysql.py::mysqlClassicInitTest::testCreation <-
test/db_test_base.py FAILED
test/test_mysql.py::mysqlConcurrencyTest::testConcurrency <-
test/db_test_base.py FAILED

I don't have mysql running, and for a while it properly skips things
then .....

Are you running a full test suite when trying to validate
run_tests, or were you following my lead and only running the
indexer stuff?

On 30/06/16 08:07, John Rouillard wrote:
> Sigh, John looks like your fix broke something else.
Argh. Serves me right for trying to push out some quick fixes. I've 
taken a bit of time to look at the problem properly and this time it was 
because I didn't look closely enough at the mark_class decorator when I 
copy and pasted it.

Details of the fix can be found in the commit message rev1c94afabb2cb

This time I promise everything should be in working order*

(* promise comes with no guarantee)

> Are you running a full test suite when trying to validate
> run_tests, or were you following my lead and only running the
> indexer stuff?

I run the full test suite using TravisCI before I push back my changes 
upstream, but that tests everything and doesn't skip anything, which is 
why I didn't pick up this problem. I also tested just test_indexer.py 
which didn't exhibit the problems we were hitting.

I've done a bit more manual testing of the tests this time, so hopefully 
it will all be good now.

On 28/06/16 22:08, John Rouillard wrote:
> Hmm, mysql IIRC tries to conenct to the db as part of have_backend()
> or some wrapper around have_backend() in the tests. If it can't
> connect it skips.

The bit of code you are interested in is at:

https://sourceforge.net/p/roundup/code/ci/1c94afabb2/tree/test/test_mysql.py#l47

which in addition to checking id the backend exists, attempts to check 
if the database exists as well, using:

https://sourceforge.net/p/roundup/code/ci/1c94afabb2/tree/roundup/backends/back_mysql.py#l98

I am not entirely convinced that this extra checking is desirable. If 
the database does not exist I would prefer that the tests fail loudly 
rather than skip silently (and potentially go unnoticed).

So my opinion would be that the logic used to determine if the the mysql 
tests are skipped should be the same as postgresql, not vice-versa.

Maybe this discussion should happen in another issue? or on the mailing 
list so others can add their opinion?

Hi John:

In message <7867032f-878b-43ed-21fe-3829e7a1b349@jerrykan.com>,
John Kristensen writes:
>John Kristensen added the comment:
>
>On 28/06/16 22:08, John Rouillard wrote:
>> Hmm, mysql IIRC tries to conenct to the db as part of have_backend()
>> or some wrapper around have_backend() in the tests. If it can't
>> connect it skips.
>The bit of code you are interested in is at:
>[...]
>which in addition to checking id the backend exists, attempts to check 
>if the database exists as well, using:
>[...]
>I am not entirely convinced that this extra checking is desirable. If 
>the database does not exist I would prefer that the tests fail loudly 
>rather than skip silently (and potentially go unnoticed).

Except that simply having the backend python libraries in place on
a system doesn't mean you have configured a database.

For example, if I am running roundup using mysql on a system that also
run some other app using postgres, and I want to test the code before
install, it will falsely fail postgres tests possibly obscuring other
failing tests that I care about.

Right now running tests with an explicit -k 'not (mysql or
postgresql)' is required. But is that guaranteed to only disable tests
that require a running copy of those two databases? Could it
accidently trap other tests?

>So my opinion would be that the logic used to determine if the the mysql 
>tests are skipped should be the same as postgresql, not vice-versa.

I see your case, but would argue for the opposite. Would this work?

Add an explicit "is database up" test designed to catch the case where
the back end is configured but there is no db backing it?

So the skip decorators will disable all mysql/postgres tests if the
backend is missing or if there is no db to connect to. However the new
PostgresIsDatabaseUp and MysqlIsDatabaseUp tests would *not* be
decorated. This way the two errors are reported if -k 'not
IsDatabaseUp' (a superset of -k PostgresIsDatabaseUp and -k
MysqlIsDatabaseUp) isn't used.

Does that fill your requirement?

>Maybe this discussion should happen in another issue? or on the mailing 
>list so others can add their opinion?

If you think my proposal needs more thought, or you want to show it to
a larger audience, feel free to toss this issue and msg references on
the list. We can open a new issue if/when this is ready for
implementation.

On 01/07/16 03:02, John Rouillard wrote:
>> John Kristensen added the comment:
>>
>> I am not entirely convinced that this extra checking is desirable. If
>> the database does not exist I would prefer that the tests fail loudly
>> rather than skip silently (and potentially go unnoticed).
>
> Except that simply having the backend python libraries in place on
> a system doesn't mean you have configured a database.
>
> For example, if I am running roundup using mysql on a system that also
> run some other app using postgres, and I want to test the code before
> install, it will falsely fail postgres tests possibly obscuring other
> failing tests that I care about.

My counter argument to this would be that each application should be 
running in its own virtualenv. Once two applications are relying on a 
common dependency it is only a matter of time before it comes back to 
bite you with version incompatibilities.

> Right now running tests with an explicit -k 'not (mysql or
> postgresql)' is required. But is that guaranteed to only disable tests
> that require a running copy of those two databases? Could it
> accidently trap other tests?

I'll say "maybe". You would probably need to just check which test names 
are matched and hope the naming of those, or new, tests stay consistent 
in the future.

An alternative may be to look at using marks to make this sort of thing 
easier:

   https://pytest.org/latest/example/markers.html

Though given the issues I've had so far with the skip markers, I'm a 
little hesitant to try and implement them and break everything again ;)

I am going to close this. I think the open issues at the end have a 
workaround so no need to fix at this time.