Issue 2550910
Created on 2016-06-26 00:17 by rouilj, last changed 2017-10-15 00:59 by rouilj.
Messages | |||
---|---|---|---|
msg5626 | Author: [hidden] (rouilj) | Date: 2016-06-26 00:17 | |
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 |
|||
msg5629 | Author: [hidden] (jerrykan) | Date: 2016-06-26 13:07 | |
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. |
|||
msg5630 | Author: [hidden] (jerrykan) | Date: 2016-06-26 14:50 | |
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. |
|||
msg5662 | Author: [hidden] (jerrykan) | Date: 2016-06-27 04:46 | |
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? |
|||
msg5664 | Author: [hidden] (rouilj) | Date: 2016-06-27 22:26 | |
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 |
|||
msg5674 | Author: [hidden] (jerrykan) | Date: 2016-06-28 05:22 | |
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 |
|||
msg5675 | Author: [hidden] (jerrykan) | Date: 2016-06-28 06:15 | |
OK, another attempt at fixing this has been committed in: https://sourceforge.net/p/roundup/code/ci/43a1f7fe39/ |
|||
msg5678 | Author: [hidden] (rouilj) | Date: 2016-06-28 12:08 | |
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. |
|||
msg5679 | Author: [hidden] (rouilj) | Date: 2016-06-28 21:22 | |
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. |
|||
msg5684 | Author: [hidden] (rouilj) | Date: 2016-06-29 22:07 | |
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? |
|||
msg5686 | Author: [hidden] (jerrykan) | Date: 2016-06-30 03:49 | |
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. |
|||
msg5687 | Author: [hidden] (jerrykan) | Date: 2016-06-30 04:00 | |
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? |
|||
msg5689 | Author: [hidden] (rouilj) | Date: 2016-06-30 17:02 | |
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. |
|||
msg5693 | Author: [hidden] (jerrykan) | Date: 2016-07-01 01:51 | |
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 ;) |
|||
msg6034 | Author: [hidden] (rouilj) | Date: 2017-10-15 00:59 | |
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. |
History | |||
---|---|---|---|
Date | User | Action | Args |
2017-10-15 00:59:17 | rouilj | set | status: open -> fixed resolution: fixed messages: + msg6034 |
2016-07-01 01:51:16 | jerrykan | set | messages: + msg5693 |
2016-06-30 17:02:15 | rouilj | set | messages: + msg5689 |
2016-06-30 04:00:23 | jerrykan | set | messages: + msg5687 |
2016-06-30 03:49:17 | jerrykan | set | messages: + msg5686 |
2016-06-29 22:07:59 | rouilj | set | messages: + msg5684 |
2016-06-28 21:22:06 | rouilj | set | messages: + msg5679 |
2016-06-28 12:08:21 | rouilj | set | messages: + msg5678 |
2016-06-28 06:15:15 | jerrykan | set | messages: + msg5675 |
2016-06-28 05:22:29 | jerrykan | set | messages: + msg5674 |
2016-06-27 22:26:17 | rouilj | set | messages: + msg5664 |
2016-06-27 04:46:38 | jerrykan | set | status: new -> open messages: + msg5662 |
2016-06-26 14:50:57 | jerrykan | set | messages: + msg5630 |
2016-06-26 13:07:48 | jerrykan | set | assignee: jerrykan messages: + msg5629 nosy: + jerrykan |
2016-06-26 00:17:04 | rouilj | create |