https://redmine.c3s.cc/https://redmine.c3s.cc/favicon.ico2019-03-13T23:54:18ZC3S Trackercollecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=28152019-03-13T23:54:18ZAlexander Blum
<ul><li><strong>Estimated time</strong> set to <i>2.00</i></li></ul> collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=30292019-03-17T08:58:28ZAlexander Blum
<ul><li><strong>Priority</strong> changed from <i>Normal</i> to <i>Hoch</i></li></ul> collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=30352019-03-17T09:07:02ZAlexander Blum
<ul><li><strong>Priority</strong> changed from <i>Hoch</i> to <i>Dringend</i></li></ul> collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32152019-06-29T15:29:52ZThomas Mielke
<ul></ul><p>functional tests now have db support again; test_login_with_right_credentials() throws exception, which is obviously not database-related (acl?).</p>
<p>unit tests still need to be fixed.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32162019-06-29T17:26:30ZThomas Mielke
<ul></ul><p><strong>Concerning the failing functional test test_login_with_right_credentials():</strong></p>
<p>User <a href="mailto:meik@c3s.cc">meik@c3s.cc</a> is obviously part of demo data, but not test data. I tried replacing it with user '<a href="mailto:1@rep.test">1@rep.test</a>' but then the assert won't fit:</p>
<p>self.assertIn('The resource was found at', res2.body)</p>
<p>No idea what the original test author intended with this. Tests should be inline-documented on a line-by-line basis...</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32172019-06-29T18:55:27ZAlexander Blum
<ul></ul><blockquote>
<p>test_login_with_right_credentials()</p>
</blockquote>
<p>The name describes the purpose, what is to be tested. So all what should<br>
be done here is exactly that: that a login with right credentials works as intended.</p>
<blockquote>
<p>self.assertIn('The resource was found at', res2.body)</p>
</blockquote>
<p>The result should be the backend html - but there is some stuff going on before the backend: there's a redirect to '/dashboard', which is redirected to '/repertoire', which is redirected to '/repertoire/dashboard' - just have a look at the views and/or your browser network request log to understand this:</p>
<ul>
<li>/dashboard will be needed, if there are more plugins.</li>
<li>The redirect from /repertoire to /repertoire/dashboard is just to have some entry point, which could be pointed elsewhere in the future (e.g. due to user settings)</li>
</ul>
<p>So, in a nutshell, the author of the code is just testing, if the request results in a redirection response via the message in the html body. Probably there's no redirect, when the authentication failed. This test could be optimized though, to be more explicit.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32182019-06-29T19:03:24ZAlexander Blum
<ul></ul><p>e.g. an obvious better way would be to test for the response code instead of possibly unreliable human readable messages.</p>
<p>If you haven't stumbled upon it yet, <a href="https://docs.pylonsproject.org/projects/webtest/en/latest/testresponse.html" class="external">this</a> is the response object you are dealing with in webtest.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32192019-06-29T19:09:02ZAlexander Blum
<ul></ul><p>ah - and the very proper way to prevent such credentials from being invalid after some time is to explicitly create an account via setUp and maybe delete it with tearDown (it should not have any side effects on other tests at least). It's dependency on some other import somewhere else is unneccessary, tests should contain themself as far as possible.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32202019-06-30T16:10:51ZThomas Mielke
<ul></ul><p>I wasn't able to get something beyond a 403 here and I'm giving up on it now:</p>
<pre>res3 = res2.follow().follow().follow()
</pre>
<p>There must be some difference between real browser requests and the WebTest framework behavior.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32212019-06-30T22:42:30ZAlexander Blum
<ul></ul><p>Thomas Mielke wrote:</p>
<blockquote>
<p>beyond a 403</p>
</blockquote>
<p>There is nothing beyond a 403, that's a 'forbidden' response. The login does not work then. Credentials? Test DB Setup?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32222019-07-01T10:14:26ZThomas Mielke
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<p>Thomas Mielke wrote:</p>
<blockquote>
<p>beyond a 403</p>
</blockquote>
<p>There is nothing beyond a 403, that's a 'forbidden' response. The login does not work then. Credentials? Test DB Setup?</p>
</blockquote>
<p>But login does work with a real browser. This is just looking for phantom bugs.</p>
<p>If the login failed, "Login failed" would have appeared as in the negative login test above this test.</p>
<p>Someone should 'fix' this test who has an agenda and sees some meaning in the way the test works.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32232019-07-01T16:59:02ZThomas Mielke
<ul></ul><p>Today I tried to make progress with unit tests. But again had no luck: there is an exception in the decorator code of transaction in models/base.py and I don't have any clue how to solve it. This is code without any inline comment.</p>
<pre> def decorator(func):
@wraps(func)
def wrapper(*args, **kwargs):
_db = Tdb._db
_user = user or 0
_retry = Tdb._retry or 0
_readonly = readonly
if 'request' in kwargs:
_readonly = not (
kwargs['request'].method
in ('PUT', 'POST', 'DELETE', 'PATCH'))
_tdbg(func, "WRAP", None, 1)
for count in range(_retry, 0, -1):
if closed():
_tdbg(func, "CONNECT")
with Transaction().start(_db, 0):
Cache.clean(_db)
pool = Pool(Tdb._db)
User = pool.get('res.user')
_context = User.get_preferences(context_only=True)
_context.update(context or {})
Transaction().start(
_db, _user, readonly=_readonly, context=_context,
close=False)
cursor = Transaction().cursor
try:
_tdbg(func, "CALL", "Try %s, Cursor %s" %
(_retry + 1 - count, id(cursor)))
result = func(*args, **kwargs)
if not _readonly:
_tdbg(func, "COMMIT", "Try %s, Cursor %s" %
(_retry + 1 - count, id(cursor)))
cursor.commit() # <-- inner exception here: "AttributeError: 'NoneType' object has no attribute 'commit'"
</pre>
<p>when called after the unit tests:</p>
<pre> cursor = Transaction().cursor
if cursor and not Transaction().cursor._conn.closed:
Cache.resets(Tdb._db)
Transaction().stop() # <-- exception on database close code lent from close_db_connection()
</pre>
<p>Why does this feel like punishment? :)</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32242019-07-03T12:45:02ZThomas Mielke
<ul><li><strong>Status</strong> changed from <i>Neu</i> to <i>In Bearbeitung</i></li><li><strong>Assignee</strong> set to <i>Alexander Blum</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>50</i></li></ul><p>In <code>Transaction.stop()</code> the <code>cursor</code> is set to <code>None</code> but is being used thereafter in the decorator function at<br>
<a href="https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/models/base.py#L201">https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/models/base.py#L201</a></p>
<p>This construct seems pretty obscure to me as I'm not familiar with any of the concepts in use here.</p>
<p>PS: don't decorators violate pythons 'make it explicit' rule?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32252019-07-04T15:06:58ZThomas Mielke
<ul></ul><p>Maybe we should use Proteus for database access in unit tests?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32262019-07-04T17:53:10ZAlexander Blum
<ul></ul><blockquote>
<p>This is just looking for phantom bugs.</p>
</blockquote>
<p>Probably not - maybe it's just, because your browser does some automagical things.</p>
<blockquote>
<p>If the login failed, "Login failed" would have appeared as in the negative login test above this test.</p>
</blockquote>
<p>What do you mean?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32272019-07-04T18:00:03ZAlexander Blum
<ul></ul><p>1.</p>
<pre>cursor.commit() # <-- inner exception here: "AttributeError: 'NoneType' object has no attribute 'commit'"
</pre>
<p>2.</p>
<pre>if cursor and not Transaction().cursor._conn.closed:
Cache.resets(Tdb._db)
Transaction().stop() # <-- exception on database close code lent from close_db_connection()
</pre>
<p>The question is, why in 1. there is no cursor anymore, although 2. explicitly checks for its existence. Obviously it gets destructed inbetween, or the check is faulty?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32282019-07-04T18:03:18ZAlexander Blum
<ul></ul><p>Thomas Mielke wrote:</p>
<blockquote>
<p>In <code>Transaction.stop()</code> the <code>cursor</code> is set to <code>None</code> but is being used thereafter in the decorator function</p>
</blockquote>
<p>Which function being decorated? Asked the other way around: why is the transaction stopped before all transactions are finished?</p>
<blockquote>
<p>PS: don't decorators violate pythons 'make it explicit' rule?</p>
</blockquote>
<p>If it would, there would be no decorators in python ;) The code gets way cleaner for certain situations using them.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32292019-07-04T18:08:04ZAlexander Blum
<ul></ul><p>Thomas Mielke wrote:</p>
<blockquote>
<p>Maybe we should use Proteus for database access in unit tests?</p>
</blockquote>
<p>Unittests in pyramid should never test the database, but the app methods. If you want to test Tdb and the model wrapper classes, it would be funny to circumvent Tdb and the model wrapper classes ;) Anyway, it's a different syntax (even less powerful) and cannot replace the tryton pool objects used in the model wrapper.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32302019-07-04T18:13:23ZAlexander Blum
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<p>why is the transaction stopped before all transactions are finished?</p>
</blockquote>
<p>More explicitly: the transaction shall not be stopped, before the transactions have finished - which means probably, that all decorated functions must have returned.</p>
<p>You might also switch on tdb debugging in development.ini (outputs to /ado/tmp/transaction.log), maybe it could help?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32312019-07-05T12:02:31ZThomas Mielke
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<blockquote>
<p>If the login failed, "Login failed" would have appeared as in the negative login test above this test.</p>
</blockquote>
<p>What do you mean?</p>
</blockquote>
<p><a href="https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71">https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71</a></p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32322019-07-05T12:05:52ZThomas Mielke
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<p>Thomas Mielke wrote:</p>
<blockquote>
<p>PS: don't decorators violate pythons 'make it explicit' rule?</p>
</blockquote>
<p>If it would, there would be no decorators in python ;) The code gets way cleaner for certain situations using them.</p>
</blockquote>
<p>If you hide all the ugly stuff, the code looks cleaner for sure! B)</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32332019-07-05T12:08:34ZAlexander Blum
<ul></ul><p>Thomas Mielke wrote:</p>
<blockquote>
<p>Alexander Blum wrote:</p>
<blockquote>
<blockquote>
<p>If the login failed, "Login failed" would have appeared as in the negative login test above this test.</p>
</blockquote>
<p>What do you mean?</p>
</blockquote>
<p><a href="https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71">https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71</a></p>
</blockquote>
<p>But it does not fail? Or what are the implications?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32342019-07-05T12:12:15ZThomas Mielke
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<p>Thomas Mielke wrote:</p>
<blockquote>
<p>Maybe we should use Proteus for database access in unit tests?</p>
</blockquote>
<p>Unittests in pyramid should never test the database, but the app methods.</p>
</blockquote>
<p>So, we should simulate access to the Company object in unit tests?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32352019-07-05T12:15:31ZThomas Mielke
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<p>Thomas Mielke wrote:</p>
<blockquote>
<p>Alexander Blum wrote:</p>
<blockquote>
<blockquote>
<p>If the login failed, "Login failed" would have appeared as in the negative login test above this test.</p>
</blockquote>
<p>What do you mean?</p>
</blockquote>
<p><a href="https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71">https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71</a></p>
</blockquote>
<p>But it does not fail? Or what are the implications?</p>
</blockquote>
<p>No, it does not fail. It just gets a 403 somewhere, but doesn't harm a real browser on its way through the redirects, finally getting to the dashboard.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32362019-07-05T12:21:15ZAlexander Blum
<ul></ul><blockquote>
<p>If you hide all the ugly stuff, the code looks cleaner for sure! B)</p>
</blockquote>
<p>:P</p>
<ul>
<li><a href="https://www.python.org/dev/peps/pep-0318/#motivation">https://www.python.org/dev/peps/pep-0318/#motivation</a></li>
<li><a href="https://www.oreilly.com/ideas/5-reasons-you-need-to-learn-to-write-python-decorators">https://www.oreilly.com/ideas/5-reasons-you-need-to-learn-to-write-python-decorators</a></li>
</ul>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32372019-07-05T12:26:53ZAlexander Blum
<ul></ul><blockquote>
<p>So, we should simulate access to the Company object in unit tests?</p>
</blockquote>
<p>Depends on what needs to be tested. If you want to test the model wrapper <a href="https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/models/company.py#L11" class="external">Company</a>, there's no point in mocking the company object. If you test another function, which just needs a company as context, it's fine to mock it - and even better, as you mock away some code, which could fail but has nothing to do with the test you are writing.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32382019-07-05T12:31:19ZAlexander Blum
<ul></ul><p>Thomas Mielke wrote:</p>
<blockquote>
<p>Alexander Blum wrote:</p>
<blockquote>
<p>Thomas Mielke wrote:</p>
<blockquote>
<p>Alexander Blum wrote:</p>
<blockquote>
<blockquote>
<p>If the login failed, "Login failed" would have appeared as in the negative login test above this test.</p>
</blockquote>
<p>What do you mean?</p>
</blockquote>
<p><a href="https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71">https://github.com/C3S/collecting_society.portal/blob/89b40d9902e102c9d0d8f9e185b1890a9d9facac/collecting_society_portal/tests/functional/login.py#L71</a></p>
</blockquote>
<p>But it does not fail? Or what are the implications?</p>
</blockquote>
<p>No, it does not fail. It just gets a 403 somewhere, but doesn't harm a real browser on its way through the redirects, finally getting to the dashboard.</p>
</blockquote>
<p>Logins with wrong credentials should definitely not give you the backend ;) I think, now I get what you mean - the login with right credentials is working, as it behaves differently than with wrong credentials.</p>
<p>Have you checked, where the redirections lead to?</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32392019-07-05T12:43:43ZThomas Mielke
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<p>Have you checked, where the redirections lead to?</p>
</blockquote>
<p>Leads to a 403 rightaway.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32402019-07-05T16:03:58ZAlexander Blum
<ul></ul><a name="TestLogintest_login_with_right_credentials"></a>
<h1 >TestLogin.test_login_with_right_credentials<a href="#TestLogintest_login_with_right_credentials" class="wiki-anchor">¶</a></h1>
<ul>
<li>I encountered, that for testing the <a href="https://github.com/C3S/c3s.ado.repertoire/blob/develop/ado/ado-do#L433" class="external">scenario_demo_data</a> is used</li>
<li>So, I don't know, which database you connected to (should have been test or test_template), but there should be no <a href="mailto:1@rep.test">1@rep.test</a> user in it (by the way, you should definitely be able to connect to the test or test_template database via tryton, it's the exact same setup as c3s database)</li>
<li>We probably use scenario_demo_data, because scenario_test_data lasts forever to run through, which would be bad both for execution time of the tests (as Jenkins recreates the whole thing every time) and for the VM resources (as it is executed on every push)</li>
<li>The scenario_demo_data is broken with 3 errors (you said, it ran through without errors, as far as I remember? This is the first thing to check in general, if something went wrong)</li>
<li>So, probably the login does not work, as the db is not setup right (have not checked it further though)</li>
<li>Just to test the login itself, I created a test db with scenario_test_data - login with <a href="mailto:1@rep.test">1@rep.test</a> and the current test code works fine there</li>
<li>Short fix: fix scenario_demo_data (could be compared with object creation in scenario_test_data, which works fine)</li>
<li>Long term goal: only scenario_master_data for the test db (no test/demo data at all) and creation of the needed context (db objects, etc) within the tests</li>
</ul>
<a name="TestHelperstest_format_currency"></a>
<h1 >TestHelpers.test_format_currency<a href="#TestHelperstest_format_currency" class="wiki-anchor">¶</a></h1>
<ul>
<li>As I <a href="https://redmine.c3s.cc/issues/701#note-19" class="external">said</a>, all Tdb.transaction() wrapped functions need to return before the Transaction().stop()</li>
<li>Within the test, the <a href="https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/tests/unit/helpers.py#L33" class="external">wrapped</a> function will not be able, to finish it's transaction, as it's transaction is <a href="https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/tests/unit/helpers.py#L72" class="external">stopped</a> before it returns.</li>
</ul>
<p>In a nutshell the wrapper works like that:</p>
<pre>@Tdb.transaction()
def somefunction(...):
pass
</pre>
<p>A call of somefunction(...) then will be equal to the following code (important parts from <a href="https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/models/base.py#L81" class="external">Tdb.transaction</a>, the rest is optimization like debugging, caching, retries, etc):</p>
<pre>if closed(): # does an "open" cursor exist?
Transaction().start(...) # if not, initiate a new connection and create one
cursor = Transaction().cursor # the cursor, accessible from everywhere via singleton Transaction()
result = somefunction(...) # execute the wrapped function
if not _readonly: # was readonly set to False via decorator arguments?
cursor.commit() # if so, commit the cursor to finally write the changes to the db
return result # return, whatever the wrapped function returned
</pre>
<p>So, if you stop the transaction inbeteeen, it will be equal to:</p>
<pre>if closed():
Transaction().start(...)
cursor = Transaction().cursor
result = somefunction(...)
'-> Transaction().stop() # our cursor is gone!
if not _readonly:
cursor.commit() # no cursor anymore!
return result
</pre>
<p>Is it now clear, what is happening here? Some other things to notice:</p>
<ul>
<li>It is not clear, why the cursor wants to be commited - no readonly=False, as far as I looked into it</li>
<li>Your code contains a lot of the same stuff in Tdb.transaction(), so your code should not be neccessary</li>
</ul>
<p>By the way, you might jump into pdb on errors by adding <code>--pdb</code>:</p>
<pre>host$ docker-compose run --use-aliases portal bash
portal$ ado-do run-tests --path src/collecting_society.portal/collecting_society_portal/tests/unit/helpers.py:TestHelpers.test_format_currency --pdb
</pre>
<p>Pdbing into Tdb.transaction, I checked, where the cursor is dropped and found out, that it never gets created in the first place, as <a href="https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/models/base.py#L122" class="external">close()</a> returns also False, when no cursor exists. This might either be a leftover from me trying to get stacked cursors to work or it assumes, that some other process is creating the first connection (like it is done, when the app framework is initialized). It could also be, that some other function depends on closed(). I have no time to test it, but you could try defining a new function to implement the expected behaviour "does an "open" cursor exist?":</p>
<pre>def open():
if Transaction().cursor and not Transaction().cursor._conn.closed
return True
return False
</pre>
<p>And use this function instead of <a href="https://github.com/C3S/collecting_society.portal/blob/develop/collecting_society_portal/models/base.py#L181" class="external">closed()</a></p>
<pre>if not open():
...
</pre>
<p>At least the test is ok then, but it needs further testing for side effects (you could try to run all other tests as your first usecase for having tests ;) )</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32412019-07-05T16:12:58ZAlexander Blum
<ul></ul><p>Ah, and also btw: To recreate the test_template db, just delete it and run some test again.</p>
<pre>ado-do db-delete test_template
</pre> collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32422019-07-06T10:46:15ZThomas Mielke
<ul><li><strong>Assignee</strong> changed from <i>Alexander Blum</i> to <i>Thomas Mielke</i></li></ul> collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32432019-07-08T13:58:46ZAlexander Blum
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<p>At least the test is ok then</p>
</blockquote>
<p>Oh, I meant, without the custom code around the test, just @Tdb.transaction() is enough.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32442019-07-08T16:46:30ZThomas Mielke
<ul></ul><p>Alexander Blum wrote:</p>
<blockquote>
<a name="TestLogintest_login_with_right_credentials"></a>
<h1 >TestLogin.test_login_with_right_credentials<a href="#TestLogintest_login_with_right_credentials" class="wiki-anchor">¶</a></h1>
<ul>
<li>I encountered, that for testing the <a href="https://github.com/C3S/c3s.ado.repertoire/blob/develop/ado/ado-do#L433" class="external">scenario_demo_data</a> is used</li>
</ul>
</blockquote>
<p>This means, for tests the demo data ist used and for the demo setup the test data is used?...</p>
<blockquote>
<ul>
<li>So, I don't know, which database you connected to (should have been test or test_template), but there should be no <a href="mailto:1@rep.test">1@rep.test</a> user in it (by the way, you should definitely be able to connect to the test or test_template database via tryton, it's the exact same setup as c3s database)</li>
</ul>
</blockquote>
<p>I also tested with wilbur webuser. I have set a breakpoint in the test code to be sure and psql-ed the test db.</p>
<blockquote>
<ul>
<li>We probably use scenario_demo_data, because scenario_test_data lasts forever to run through, which would be bad both for execution time of the tests (as Jenkins recreates the whole thing every time) and for the VM resources (as it is executed on every push)</li>
</ul>
</blockquote>
<p>There should be a lite version of test data with lesser db entries.</p>
<blockquote>
<ul>
<li>The scenario_demo_data is broken with 3 errors (you said, it ran through without errors, as far as I remember? This is the first thing to check in general, if something went wrong)</li>
</ul>
</blockquote>
<p>I didn't build the test data till now. Confirm that 3 of 213 tests failed during db generation.</p>
<blockquote>
<ul>
<li>So, probably the login does not work, as the db is not setup right (have not checked it further though)</li>
</ul>
</blockquote>
<p>Company and Webuser don't seem affected.</p>
<blockquote>
<ul>
<li>Just to test the login itself, I created a test db with scenario_test_data - login with <a href="mailto:1@rep.test">1@rep.test</a> and the current test code works fine there</li>
</ul>
</blockquote>
<p>Interesting.</p>
<blockquote>
<ul>
<li>Short fix: fix scenario_demo_data (could be compared with object creation in scenario_test_data, which works fine)</li>
</ul>
</blockquote>
<p>The fix is to fix! :D</p>
<p><code>File "/ado/etc/scenario_demo_data.txt", line 143, in scenario_demo_data.txt<br>
Failed example:<br>
creative.save()<br>
Exception raised:<br>
Traceback (most recent call last):<br>
File "/usr/lib/python2.7/doctest.py", line 1315, in __run<br>
compileflags, 1) in test.globs<br>
File "<doctest scenario_demo_data.txt[53]>", line 1, in <module><br>
creative.save()<br>
File "/usr/local/lib/python2.7/dist-packages/proteus/__init__.py", line 695, in save<br>
self._proxy.write([self.id], values, context)<br>
File "/usr/local/lib/python2.7/dist-packages/proteus/config.py", line 162, in __call__<br>
result = rpc.result(meth(*args, **kwargs))<br>
File "/ado/src/trytond/trytond/model/modelsql.py", line 829, in write<br>
cls._validate(sub_records, field_names=all_field_names)<br>
File "/ado/src/trytond/trytond/model/modelstorage.py", line 1080, in _validate<br>
error_args=error_args)<br>
File "/ado/src/trytond/trytond/error.py", line 74, in raise_user_error<br>
raise UserError(error)<br>
UserError: ('UserError', ('selection_validation_record', ''))</code></p>
<p>Havent touched the creative code for ages. Haven't we?</p>
<blockquote>
<a name="TestHelperstest_format_currency"></a>
<h1 >TestHelpers.test_format_currency<a href="#TestHelperstest_format_currency" class="wiki-anchor">¶</a></h1>
<ul>
<li>As I <a href="https://redmine.c3s.cc/issues/701#note-19" class="external">said</a>, all Tdb.transaction() wrapped functions need to return before the Transaction().stop()</li>
</ul>
</blockquote>
<p>Ok, so I moved the Tdb.init() and transaction start code into setUpClass of TestHelpers to make sure the transaction is initialized before the decorator. I now get get:</p>
<p><code>Traceback (most recent call last):<br>
File "/ado/src/collecting_society.portal/collecting_society_portal/tests/unit/models/base.py", line 32, in test_autenticate_user<br>
webu = WebUser.authenticate("alf_imp@c3s.cc", "cc")<br>
File "/ado/src/collecting_society.portal/collecting_society_portal/models/web_user.py", line 130, in authenticate<br>
users = cls.get().search([('email', 'ilike', cls.escape(email))])<br>
File "/ado/src/trytond/trytond/model/modelsql.py", line 969, in search<br>
pool = Pool()<br>
File "/ado/src/trytond/trytond/pool.py", line 53, in __new__<br>
database_name = Transaction().cursor.database_name<br>
AttributeError: 'NoneType' object has no attribute 'database_name'</code></p>
<blockquote>
<p>Is it now clear, what is happening here?</p>
</blockquote>
<p>No. For example: Why do I have to provide the transaction code twice?</p>
<p>Sorry, but I didn't understand what you wanted to illustrate with your code samples.</p>
<p>Please mind my brain capacity! :)</p>
<p>I might be able to work on well designed Python code. But don't expect me to delve into flawed code not even you were able to understand completely. 8-/</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32452019-07-08T17:29:44ZAlexander Blum
<ul></ul><blockquote>
<p>for tests the demo data ist used and for the demo setup the test data is used?...</p>
</blockquote>
<p>For the normal c3s setup, scenario_test_data is used, so I would not call it "demo setup".</p>
<p>The purpose of scenario_demo_data was to provide a minimal set of tests for the database in a scenario style file (telling the story of what is happening within the comments) and to provide a minimal data set to be able to present the capabilities of the app ("demo").</p>
<p>In lack of some standardized, possibly standalone import script I wrote scenario_test_data, as we needed some more functional, parametrized import style (no story telling here) to have a lot of data for human testing. Once we have some import script, scenario_test_data should be merged into that.</p>
<p>To bootstrap the pyramid tests and get some coverage in a fast way, it just happened, that scenario_demo_data was chosen to have something to work with.</p>
<blockquote>
<p>I also tested with wilbur webuser. I have set a breakpoint in the test code to be sure and psql-ed the test db.</p>
</blockquote>
<p>I can't tell you, what is wrong with the database - but as scenario_test_data works, the demo setup is the problem here for sure.</p>
<blockquote>
<p>There should be a lite version of test data with lesser db entries.</p>
</blockquote>
<p>As I said, this is suboptimal, as the tests should not depend on external factors like previously imported data sets, but should create/delete it.</p>
<p>But it could be a workaround. For now, you could just rename scenario_demo_data to have a backup of it, copy the scenario_test_data to scenario_demo_data and adjust the parameter for the entry numbers on top of the file.</p>
<blockquote>
<p>Havent touched the creative code for ages. Haven't we?</p>
</blockquote>
<p>The creative code went into repertoire, and we probably did a lot of changes to it.</p>
<blockquote>
<blockquote>
<a name="TestHelperstest_format_currency"></a>
<h1 >TestHelpers.test_format_currency<a href="#TestHelperstest_format_currency" class="wiki-anchor">¶</a></h1>
<p>Ok, so I moved the Tdb.init() and transaction start code into setUpClass of TestHelpers to make sure the transaction is initialized before the decorator.</p>
</blockquote>
</blockquote>
<p>You don't need any of the Transaction code, just the Tdb.init() stuff, which was there already.</p>
<p>And you need to fix the wrong 'closed' condition within Tdb.transaction() as stated above.</p>
<blockquote>
<blockquote>
<p>Is it now clear, what is happening here?</p>
</blockquote>
<p>No. For example: Why do I have to provide the transaction code twice?</p>
</blockquote>
<p>You don't need to.</p>
<blockquote>
<p>Sorry, but I didn't understand what you wanted to illustrate with your code samples.</p>
</blockquote>
<p>To provide you with a complete understanding of what @Tdb.transaction() is doing.</p>
<p>We should switch to an audio channel, then I might be able to explain better.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32732019-09-16T17:40:46ZThomas Mielke
<ul></ul><p>I tried around another hour without any success.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32742019-09-17T14:53:17ZThomas Mielke
<ul></ul><p>I tried around another hour without any success.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=32762019-09-22T02:56:09ZAlexander Blum
<ul><li><strong>Status</strong> changed from <i>In Bearbeitung</i> to <i>Erledigt</i></li><li><strong>% Done</strong> changed from <i>50</i> to <i>100</i></li></ul><p>Applied in changeset commit:collecting_society_portal_repertoire|c55d488d3ccf4930cb5ad5c181de23e8ccab32e6.</p>
collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=34462019-10-08T13:37:11ZAlexander Blum
<ul><li><strong>Target version</strong> changed from <i>2) Testing phase II</i> to <i>Repertoire 2) Testing phase II</i></li></ul> collecting_society - Webfrontend #701: Fix testshttps://redmine.c3s.cc/issues/701?journal_id=39012019-10-08T13:52:38ZAlexander Blum
<ul><li><strong>Project</strong> changed from <i>repertoire</i> to <i>collecting_society</i></li></ul>