Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Run tests under PostgreSQL #3423

Merged
merged 5 commits into from
Aug 13, 2018
Merged

Run tests under PostgreSQL #3423

merged 5 commits into from
Aug 13, 2018

Conversation

hawkowl
Copy link
Contributor

@hawkowl hawkowl commented Jun 22, 2018

No description provided.

@hawkowl hawkowl changed the title [WIP] Run tests under PostgreSQL Run tests under PostgreSQL Aug 1, 2018
@hawkowl hawkowl requested a review from a team August 1, 2018 13:42
tests/utils.py Outdated
@@ -100,11 +144,25 @@ def setup_test_homeserver(name="test", datastore=None, config=None, reactor=None
"database": ":memory:",
"cp_min": 1,
"cp_max": 1,
"check_same_thread": False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we setting this?

tests/utils.py Outdated
# the template database we generate in setupdb()
if datastore is None and isinstance(db_engine, PostgresEngine):
db_conn = db_engine.module.connect(
"dbname=synapse_base user=%s" % (POSTGRES_USER,))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use keyword args here, rather than connection strings: http://initd.org/psycopg/docs/module.html#psycopg2.connect

tests/utils.py Outdated
pgconfig = {
"name": "psycopg2",
"args": {
"database": "synapse_base",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call this something like _synapse_unit_tests_temp_base or something, I can't see it far too easily dropping valid databases.

.travis.yml Outdated
@@ -5,9 +5,13 @@ language: python
cache: pip

before_script:
- psql -c 'create database synapse_base;' -U postgres
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary if we're creating it manually anyway?

@hawkowl hawkowl requested a review from a team August 10, 2018 15:05
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm modulo the nit about the missing docstring.

However, what would be really good would be some words for developers about getting the tests running: what they'll need on their box (a running postgres, and a user with permission to create and drop databases, presumably); it would also be worth documenting the names of said databases (if nothing else because I want to know that it isn't going to drop my main synapse database :)

Feel free to stick the docs in a separate PR though.



@defer.inlineCallbacks
def setup_test_homeserver(
name="test", datastore=None, config=None, reactor=None, **kargs
cleanup_func, name="test", datastore=None, config=None, reactor=None, **kargs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can haz docstring for cleanup_func?

@richvdh
Copy link
Member

richvdh commented Aug 11, 2018

However, what would be really good would be some words for developers about getting the tests running

Possibly in CONTRIBUTING.rst?

@hawkowl hawkowl merged commit 99dd975 into develop Aug 13, 2018
@hawkowl hawkowl deleted the hawkowl/pg-tests branch August 13, 2018 06:47
richvdh added a commit that referenced this pull request Aug 22, 2018
Features
--------

- Add support for the SNI extension to federation TLS connections. Thanks to @vojeroen! ([\#3439](#3439))
- Add /_media/r0/config ([\#3184](#3184))
- speed up /members API and add `at` and `membership` params as per MSC1227 ([\#3568](#3568))
- implement `summary` block in /sync response as per MSC688 ([\#3574](#3574))
- Add lazy-loading support to /messages as per MSC1227 ([\#3589](#3589))
- Add ability to limit number of monthly active users on the server ([\#3633](#3633))
- Support more federation endpoints on workers ([\#3653](#3653))
- Basic support for room versioning ([\#3654](#3654))
- Ability to disable client/server Synapse via conf toggle ([\#3655](#3655))
- Ability to whitelist specific threepids against monthly active user limiting ([\#3662](#3662))
- Add some metrics for the appservice and federation event sending loops ([\#3664](#3664))
- Where server is disabled, block ability for locked out users to read new messages ([\#3670](#3670))
- set admin uri via config, to be used in error messages where the user should contact the administrator ([\#3687](#3687))
- Synapse's presence functionality can now be disabled with the "use_presence" configuration option. ([\#3694](#3694))
- For resource limit blocked users, prevent writing into rooms ([\#3708](#3708))

Bugfixes
--------

- Fix occasional glitches in the synapse_event_persisted_position metric ([\#3658](#3658))
- Fix bug on deleting 3pid when using identity servers that don't support unbind API ([\#3661](#3661))
- Make the tests pass on Twisted < 18.7.0 ([\#3676](#3676))
- Don’t ship recaptcha_ajax.js, use it directly from Google ([\#3677](#3677))
- Fixes test_reap_monthly_active_users so it passes under postgres ([\#3681](#3681))
- Fix mau blocking calulation bug on login ([\#3689](#3689))
- Fix missing yield in synapse.storage.monthly_active_users.initialise_reserved_users ([\#3692](#3692))
- Improve HTTP request logging to include all requests ([\#3700](#3700))
- Avoid timing out requests while we are streaming back the response ([\#3701](#3701))
- Support more federation endpoints on workers ([\#3705](#3705), [\#3713](#3713))
- Fix "Starting db txn 'get_all_updated_receipts' from sentinel context" warning ([\#3710](#3710))
- Fix bug where `state_cache` cache factor ignored environment variables ([\#3719](#3719))
- Fix bug in v0.33.3rc1 which caused infinite loops and OOMs ([\#3723](#3723))
- Fix bug introduced in v0.33.3rc1 which made the ToS give a 500 error ([\#3732](#3732))

Deprecations and Removals
-------------------------

- The Shared-Secret registration method of the legacy v1/register REST endpoint has been removed. For a replacement, please see [the admin/register API documentation](https://github.com/matrix-org/synapse/blob/master/docs/admin_api/register_api.rst). ([\#3703](#3703))

Internal Changes
----------------

- The test suite now can run under PostgreSQL. ([\#3423](#3423))
- Refactor HTTP replication endpoints to reduce code duplication ([\#3632](#3632))
- Tests now correctly execute on Python 3. ([\#3647](#3647))
- Sytests can now be run inside a Docker container. ([\#3660](#3660))
- Port over enough to Python 3 to allow the sytests to start. ([\#3668](#3668))
- Update docker base image from alpine 3.7 to 3.8. ([\#3669](#3669))
- Rename synapse.util.async to synapse.util.async_helpers to mitigate async becoming a keyword on Python 3.7. ([\#3678](#3678))
- Synapse's tests are now formatted with the black autoformatter. ([\#3679](#3679))
- Implemented a new testing base class to reduce test boilerplate. ([\#3684](#3684))
- Rename MAU prometheus metrics ([\#3690](#3690))
- add new error type ResourceLimit ([\#3707](#3707))
- Logcontexts for replication command handlers ([\#3709](#3709))
- Update admin register API documentation to reference a real user ID. ([\#3712](#3712))
@matoro
Copy link

matoro commented Jan 16, 2019

Since the reference on this does not seem to have made it into the documentation, could you specify how to run these tests against an existing postgres installation? Currently the following:

SYNAPSE_POSTGRES=True SYNAPSE_POSTGRES_USER=postgres python -m twisted.trial -ex tests

tries to connect to the database under the running user, resulting in the following:

tests.api.test_auth
  AuthTestCase
    test_blocking_mau ... Traceback (most recent call last):
  File "/var/tmp/portage/net-im/synapse-0.34.1.1/work/synapse-0.34.1.1/tests/api/test_auth.py", line 42, in setUp                                                                                                          
    self.hs = yield setup_test_homeserver(self.addCleanup, handlers=None)
  File "/usr/lib/python3.6/site-packages/twisted/internet/defer.py", line 1418, in _inlineCallbacks
    result = g.send(result)
  File "/var/tmp/portage/net-im/synapse-0.34.1.1/work/synapse-0.34.1.1/tests/utils.py", line 298, in setup_test_homeserver                                                                                                 
    hs.setup()                                                                                                                                                                                                             
  File "/var/tmp/portage/net-im/synapse-0.34.1.1/work/synapse-0.34.1.1/synapse/server.py", line 210, in setup
    with self.get_db_conn() as conn:
  File "/var/tmp/portage/net-im/synapse-0.34.1.1/work/synapse-0.34.1.1/synapse/server.py", line 388, in get_db_conn                                                                                                        
    db_conn = self.database_engine.module.connect(**db_params)
  File "/usr/lib/python3.6/site-packages/psycopg2/__init__.py", line 130, in connect
    conn = _connect(dsn, connection_factory=connection_factory, **kwasync)                                                                                                                                                 
psycopg2.OperationalError: FATAL:  role "portage" does not exist

I based this command off the env variables found here: https://github.com/matrix-org/synapse/blob/master/tests/utils.py#L45

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants