search

FMCorz / mdk

Moodle Development Kit. A collection of tools meant to make developers' lives easier.
GNU General Public License v3.0
85 stars 47 forks source link

mdk theme in config.php - breaks behat suites #147

Closed mudrd8mz closed 5 years ago

mudrd8mz commented 7 years ago

I just ran to this in moodle 3.2 where I was experimenting with behat executed against multiple themes (boost and clean) as per https://docs.moodle.org/dev/Running_acceptance_test#Running_behat_with_specified_theme_.28Since_Moodle_3.2.29

My problem was that even with --suite=clean the tests were still executed under boost. I realized that $CFG->theme is hard-set in config.php to 'boost' as a result of my previous executions of the mdk theme alias.

I am quite used to changing the theme quickly and easily via mdk. So to avoid issues with behat, it would help if mdk set the theme via DB config table rather than via config.php file.

Or at least, can you suggest an actual command that I could use instead of the current (default) alias that would behave that way? Thanks in advance.

mudrd8mz commented 7 years ago

Just ran into the same issue again. Thanks @danpoltawski for reminding me about this (my!) issue ;-)

mudrd8mz commented 7 years ago

This can be simply solved once https://tracker.moodle.org/browse/MDL-57896 is integrated

mudrd8mz commented 5 years ago

@FMCorz I will appreciate your guide here. What I would like to have is:

  1. Remove the default alias for mdk theme from config-dist.php
  2. Introduce a new standard command mdk theme that would 2.1 For Moodle 3.3 and higher, make use of php admin/cli/cfg.php --name=theme to get or set the current theme 2.2 For earlier versions, it would either do the low level DB access, or simply did nothing but informing the user it needs Moodle 3.3 and higher to function (as I assume not many mdk users still actively develop for 3.2 and lower versions).

If you are happy with such a plan, I can eventually start looking at the solution. One thing that was not clear to me is, what would happen when a new standard command is added yet there is an alias with the same name existing.

Thanks in advance for your hints.

FMCorz commented 5 years ago

Hi David,

The alias was suggested and submitted by @andrewnicols. While I use it every now and then, I don't feel that strongly about it.

However, Moodle is meant to behave properly even when properties are set in the config file. One could argue that the real issue is with Behat ignoring a parameter that was naturally set for Moodle to use. If Behat relies on the value in the database vs. the value returned by get_config, then Behat is doing it wrong in my opinion.

If Behat really can't cope with a value in the configuration file, then the first thing I would suggest is to use add a check to admin/tool/behat/cli/run.php which would ensure that the --suite parameter does not conflict with the config values.

All that aside, I don't think MDK requires to have an alias or command for switching the theme, as the alias demonstrates, it's fairly easy to do by hand, or to alias in a shell. You could consider developing an additional command for setting configuration values (DB ones), but a) it would only be a wrapper for the Moodle CLI, b) it can certainly be achieved by other tools such as https://github.com/tmuras/moosh.

Another solution is to build this into mdk behat, which would benefit from being compatible with themes and could set/unset the config file values if need be. (#163)

mudrd8mz commented 5 years ago

Thanks. I have never thought I would consider this as a Behat's fault. Configuration parameters set in the config.php have always been considered "hard-coded" and they override anything set in the DB by design. All I wanted to say is that having the theme hard-coded in config.php naturally breaks theme switching - be it the user or Behat attempting to do it. So I stopped using the alias, and I assumed others are hitting this problem, too. Apparently not. Please feel free to close.

andrewnicols commented 5 years ago

I have raised https://tracker.moodle.org/browse/MDL-63607 I recommend closing this issue.

Moodle's behat setup is simply not compatible with theme specified in config.php. I don't know why it is a whitelisted setting because it simply will not work. The behat suite informs behat as to which scenarios, features, and overrides to use, but it is not (currently) possible to choose the suite based on the config theme. If a theme is specified in config, and it does not match the behat arg, then it will cause test failures.

Selection of theme for behat should be entirely via the --suite argument.

Recommend closing this issue.

FMCorz commented 5 years ago

Thanks guys!