search

adaptlearning / adapt-contrib-boxMenu

A simple contributed menu based upon a grid system
GNU General Public License v3.0
2 stars 29 forks source link

BoxMenu tests are giving a false positive for menu's on the course #202

Closed cahirodoherty-learningpool closed 3 weeks ago

cahirodoherty-learningpool commented 1 month ago

Subject of the issue/enhancement/features

https://github.com/adaptlearning/adapt-contrib-boxMenu/blob/master/test/e2e/adapt-contrib-boxMenu.cy.js#L6 This building of the boxMenus array makes the incorrect assumption that all courses and menus are of the BoxMenu type. This has worked for the default course for testing but proves an obstacle to tests that run on content that is built with a custom menu.

The real issue here is the failure of the first array setting as Core/Framework does not seem to apply this setting as intended https://github.com/adaptlearning/adapt-contrib-boxMenu/blob/master/test/e2e/adapt-contrib-boxMenu.cy.js#L4

I propose we remove the catchall case that currently breaks custom menu testing and also fix the _component attribute setting in Core/Framework so that boxMenu tests can be run

oliverfoster commented 1 month ago

The real issue here is the failure of the first array setting as Core/Framework does not seem to apply this setting as intended

These settings are for multiple menu courses. It is for the user to apply those properties in order to choose which menu should be used at the course or contentObject level, similar to _component on the components.json.

There is otherwise no way to determine which menu is being used in the course, except only that a single menu will usually be included in the compilation process, this then becomes the default menu of the course for all contentObject menus (course, submenus etc).

I'm assuming that you have multiple menus in a single project folder but only use one menu per course? The config.json:build.includes directive chooses which menu to include at compile time but the tests runner is not aware of the includes directive from the config.json:build, such that boxMenu tests are executed when boxMenu isn't the included menu?

cahirodoherty-learningpool commented 1 month ago

Thanks for the input @oliverfoster The problem arrises when using a non-boxMenu menu in the course. While the first array setting is (correctly) empty as nothing in the data satisfies the filter condition, the second filter assumes all course/menu items are using boxMenu, even when they aren't. Then the tests will run and likely fail forEach "boxMenu" when the content on the page doesn't match the expected

oliverfoster commented 1 month ago

The problem arrises when using a non-boxMenu menu in the course.

Another menu alongside or instead of a boxMenu? Is it a multimenu course with boxMenu or a single menu course without boxMenu?

cahirodoherty-learningpool commented 1 month ago

A single menu course. That menu is a custom menu, not boxMenu

oliverfoster commented 1 month ago

The boxMenu tests should not be running, why are they running?

cahirodoherty-learningpool commented 1 month ago

I think the current test.js setup is such that all tests run, regardless of which plugins are included in the course. Or am I mistaken there?

oliverfoster commented 1 month ago

Considering that the tests rely on the course data to perform, it probably should only run the tests from the plugins included in that course.

The config.json:build.includes directive chooses which menu to include at compile time but the tests runner is not aware of the includes directive from the config.json:build

We should fix this.

cahirodoherty-learningpool commented 3 weeks ago

Issue resolved by https://github.com/adaptlearning/adapt_framework/issues/3595