add test for pgtest argument of TemporaryProfileManager
#3486
Conversation
this adds a test (running only on travis) for the `pgtest` argument of the TemporaryProfileManager. Furthermore, the `pgtest` argument is removed from internal functions of the TemporaryProfilemanager and treated as an instance variable instead.
ltalirz
force-pushed
the
issue_3482_pgtest_argument
branch
from
98d0f20
to
5cb270f
Compare
|
For reviewers: the new test is actually in |
| self.assertIsNone(self.profile_manager.root_dir) | ||
| self.assertIsNone(self.profile_manager.pg_cluster) | ||
| # pg_ctl is what PGTest also looks for (although it might be more clever) | ||
| pgtest = {'pg_ctl': which('pg_ctl')} |
There was a problem hiding this comment.
Maybe we should use the which from pgtest? And/or add another test that tests what we expect with which from AiiDA is the same that pgtest expects. There is a tight connection between AiiDA and pgtest now, but that might change.
There was a problem hiding this comment.
Maybe we should use the which from pgtest?
Since pgtest is a bit more clever than the standard "which", we could do that.
I'll make the change.
And/or add another test that tests what we expect with which from AiiDA is the same that pgtest expects.
I don't think that needs to be the case, and it's not what I'm testing here - I'm just trying to see of the TemporaryProfileManager works if I give it some valid path to pg_ctl (could be a different one than the one autodiscovered by pgtest).
There was a problem hiding this comment.
My main question for you was to check whether you are ok with me removing the pgtest argument from the internal functions of the TemporaryProfileManager (since I guess you were the main user) - I guess as long as the functionality is still accessible from the constructor, it is fine with you, right?
There was a problem hiding this comment.
Yes, absolutely. I was using it from the test_manager, which run the constructor I think, so for me this would work nicely.
|
This is ready for review |
|
@espenfl Please have another look through this when you find the time. Cheers! |
this adds a test (running only on travis) for the
pgtestargument ofthe
TemporaryProfileManager.Furthermore, the
pgtestargument is removed from internal functions ofthe
TemporaryProfileManagerand treated as an instance variable instead.