Issue 7112 - Add centralised conftest

Issue 7112 - Add centralised conftest

[Tudor Avram, 2018-11-08 19:40:17]

Hi Vasily, 

I made the change as we discussed and I'll leave it as is for now. 

Let me know what you think.

Thanks,
Tudor.

[Vasily Kuznetsov, 2018-11-13 15:55:31]

Hi Tudor,

In general the code is good but I'm concerned about the layout. Now all the
fixtures from sitescripts/extensions tests are separated from the test and live
in conftest/sitescripts/extensions. This seems kind of suboptimal. Could they
stay next to the tests somehow and then we would import them using the
pytest_plugins thing? Also, what do you think about putting conftest.py into
/tests instead of /conftest? I'm a bit worried that /conftest directory at the
top level could be confusing for people who are not pytest gurus :D

Cheers,
Vasily

[Tudor Avram, 2018-11-14 16:09:28]

Hi Vasily,

Addressed your comments. Let me know if there's anything else that you'd like me
to change.

Tudor.

[Vasily Kuznetsov, 2018-11-14 16:34:40]

Hi Tudor!

Looks good. Just one nit about use of relative path (see below).

Also it seems that the patch doesn't apply on top of the master
(sitescripts/extensions/test/utils.py gets rejected) -- not sure if this is on
my side. Does the patch apply for you?

Cheers,
Vasily

https://codereview.adblockplus.org/29940559/diff/29942555/sitescripts/extensi...
File sitescripts/extensions/test/utils.py (right):

https://codereview.adblockplus.org/29940559/diff/29942555/sitescripts/extensi...
sitescripts/extensions/test/utils.py:29: return
py.path.local('sitescripts/extensions/test/')
I think it's better to use the approach based on __file__ otherwise the test
behaviour starts depending on current directory, which is not great.

[Tudor Avram, 2018-11-14 17:00:19]

Hi Vasily,

Addressed your last comment.

Tudor.

https://codereview.adblockplus.org/29940559/diff/29942555/sitescripts/extensi...
File sitescripts/extensions/test/utils.py (right):

https://codereview.adblockplus.org/29940559/diff/29942555/sitescripts/extensi...
sitescripts/extensions/test/utils.py:29: return
py.path.local('sitescripts/extensions/test/')
On 2018/11/14 16:34:40, Vasily Kuznetsov wrote:
> I think it's better to use the approach based on __file__ otherwise the test
> behaviour starts depending on current directory, which is not great.

Done.

[Vasily Kuznetsov, 2018-11-14 17:09:15]

LGTM