-
Notifications
You must be signed in to change notification settings - Fork 191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Accept arguments to aiida_profile #3481
Conversation
In the second commit a factory function have been used, which makes it possible to pass arguments. However, when passing e.g. Is this something we can run with? I have heard rumors that factory functions obfuscate the test printouts in case there is something amiss, but have not yet seen this myself (or cared to much about it). |
It seems @greschd solved this type of problem by using a configuration file for tests instead. If you ask me, I would lean towards the configuration file approach (see relevant links here) - adding arguments makes both the definition and the usage of these fixtures rather complicated. Perhaps @greschd would be willing to come up with a minimal PR that adds support for such a configuration file? |
Anyhow, it's not clear to me what the best solution is - perhaps it is not possible to keep the fixtures simple and configurable enough for everybody at the same time. Another approach is to say:
and suggest to developers who have particular needs write their own fixtures using the manager (after all, the fixtures themselves are very little code). Perhaps @chrisjsewell or @sphuber also have some ideas. |
Happy to take a look at your solution @greschd for instance for the With respect to complexity I do not think that is a problem as long as we support Having said that, I am happy with other solutions and this was just a suggestion following the lines we had before and the newly introduced concept. I think in principle we should avoid factory functions if we can, but if this is the only solution to offer parameters to |
@espenfl In your use case, would different developers use different This particular use case seems somewhat different from the things I proposed in #3473 (computer & code setup) in that it determines how the DB itself is set up, whereas the other things happen after that initialization step. That doesn't necessarily mean that we have to treat it in a different way, though. |
@greschd Yes and no. One could also envision testing on different Postgresql versions, but let us leave that option for now. Also such a test belongs to Also, opening up for a config file is certainly maybe the best option. I totally missed that this was considered. Sure, let us just open for a Closing this now. |
An initial solution to how to pass arguments to the recently introduced
aiida_profile
. The problem with this approach is that we on the plugin side relies on fixtures, which typically use theaiida_profile
fixture as well. Since there is no way, using this approach to pass arguments from one fixture to another (say if we want to setpg_ctl
in thepgtest
argument toaiida_profile
).We would thus severely limit the usability of fixtures on the plugin side.