Upgrade extension to CKAN 2.11 #9
Conversation
| @pytest.fixture | ||
| def announcement_migrate(): | ||
|
|
||
| _apply_alembic_migrations() |
There was a problem hiding this comment.
@avdata99 I'm not exactly sure what's going on here. Is the migrate_db_for fixture broken in CKAN 2.11?
There was a problem hiding this comment.
I can't manage to find the problem @pdelboca
I added the with_plugins fixture and now it's fixed
There was a problem hiding this comment.
@avdata99 I suggest to change this fixture for the following one:
import pytest
@pytest.fixture
def clean_db(reset_db, migrate_db_for):
reset_db()
migrate_db_for("announcements")The with_plugins is necessary to force test to use the ckan.plugins definition of the test.ini file. Without the fixture, it will be ignored.
Unit testing with CKAN is in my opinion confusing. There is a whole discussion about it, while it didn't got anywhere I think it documents what's going on when testing CKAN. I will definitely recommend making a 🧉 and read it.
There was a problem hiding this comment.
@avdata99 I have been debugging and there is a nasty bug going around.
I was able to narrow it to the with_request_context fixture that it is cleaning the current_user object for some reason. If you remove the with_request_context fixture the index view will have a proper user logged in.
Note: The Flask's TestClient should create the request context when executing app.get() so I don't see the need of having it. Usually with_request_context is used when we want to test actions that expects a request without using the TestClient, but instead of relying on Flask's api we have this extra custom fixture.
|
Done with the proposed changed. Looks good now. |
No description provided.