Skip to content

behat: configure during step; javascript is unnecessary - #710

Merged
smbader merged 1 commit into
mainfrom
fix/behat-configure
Apr 16, 2026
Merged

behat: configure during step; javascript is unnecessary#710
smbader merged 1 commit into
mainfrom
fix/behat-configure

Conversation

@jrchamp

@jrchamp jrchamp commented Apr 14, 2026

Copy link
Copy Markdown
Owner

At this time, the view_meeting Behat test does not require the @javascript tag and should therefore not include it. The generator class was setting configuration, which prevented values set in the Behat feature file from being used.

Fun side note: the mod_zoom plugin is saving all of its configuration settings using the outdated naming convention (without the mod_ prefix). Moodle's developer documentation has included a note about this since May 2015:

new activity modules should use 'mod_mymodule/setting' instead of 'mymodule/setting'

Looks like it was primarily using the modern frankenstyle name until #214. Meanwhile, look how naive I was in early 2021: #180 (comment) 😅

At this time, the view_meeting Behat test does not require the
@javascript tag and should therefore not include it. The generator class
was setting configuration, which prevented values set in the Behat
feature file from being used.

Fun side note: the mod_zoom plugin is saving all of its configuration
settings using the outdated naming convention (without the mod_ prefix).

Reference: https://moodledev.io/docs/5.2/apis/subsystems/admin
"activity modules should use 'mod_mymodule/setting' instead of 'mymodule/setting'"
@jrchamp
jrchamp requested a review from a team April 14, 2026 11:15
@jrchamp jrchamp self-assigned this Apr 14, 2026
@jrchamp jrchamp added the bug Fixes problems or reduces technical debt label Apr 14, 2026
@jrchamp

jrchamp commented Apr 14, 2026

Copy link
Copy Markdown
Owner Author

Performance comparison based on recent CI runs for arguably the simplest Behat test ever (something to be proud of!):

  • With @javascript: 7.2-9.1 seconds
  • Without @javascript: 4.5-6.6 seconds

@cbounphengsy cbounphengsy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting the config values in a more Behat way seems like the logical move.

@smbader
smbader merged commit 3942d6c into jrchamp:main Apr 16, 2026
6 checks passed
@jrchamp
jrchamp deleted the fix/behat-configure branch April 17, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes problems or reduces technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants