Skip to content

Restore possibility to change keep_session param.#40

Merged
icemac merged 3 commits into
masterfrom
icemac-patch-1
Feb 17, 2020
Merged

Restore possibility to change keep_session param.#40
icemac merged 3 commits into
masterfrom
icemac-patch-1

Conversation

@icemac
Copy link
Copy Markdown
Member

@icemac icemac commented Nov 27, 2019

Before version 1.2 it was possible to change the value of keep_session for certain tests.
We used keep_session=True to ease unittests which are not forced to re-fetch objects from database after a commit. On the other hand selenium tests should behave more like an actual web server which does not keep the session.
So we switched to keep_session=False for some tests with code like this:
https://github.com/gocept/risclog.sqlalchemy/blob/44fe2494337b41e0bc3d5602ebbd7493918d841d/src/risclog/sqlalchemy/fixtures.py#L6

This is no longer possible since zope.sqlalchemy 1.2.
With this commit the ability to access the ZopeTransactionEvents instance is restored.

Before version 1.2 it was possible to change the value of `keep_session` for certain tests.
We used `keep_session=True` to ease unittests which are not forced to re-fetch objects from database after a commit. On the other hand selenium tests should behave more like an actual web server which does not keep the session.
So we switched to `keep_session=False` for some tests with code like this:
https://github.com/gocept/risclog.sqlalchemy/blob/44fe2494337b41e0bc3d5602ebbd7493918d841d/src/risclog/sqlalchemy/fixtures.py#L6

This is no longer possible since zope.sqlalchemy 1.2.
With this commit the ability to access the `ZopeTransactionEvents` instance is restored.
@icemac icemac added the bug label Nov 27, 2019
@icemac icemac self-assigned this Nov 27, 2019
@icemac icemac requested a review from mgedmin November 27, 2019 17:28
@icemac
Copy link
Copy Markdown
Member Author

icemac commented Nov 27, 2019

Could this be a possible solution where I should invest to fix the tests or does anyone think this is a bad idea.

Copy link
Copy Markdown
Member

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

LGTM, but please document the return value in the docstring

@icemac icemac merged commit ca3a094 into master Feb 17, 2020
@icemac icemac deleted the icemac-patch-1 branch February 17, 2020 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants