Skip to content
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

Chore: Added drawer extension e2e tests (Issue/3470) #3471

Closed
wants to merge 3 commits into from

Conversation

lemmyadams
Copy link
Contributor

Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@@ -0,0 +1,63 @@
import Course from '../../../src/course/en/course.json'
Copy link
Member

Choose a reason for hiding this comment

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

Interesting... very interesting...

The tests can be run against a specified build output folder, which wouldn't have a src/course but an {outputdir}/{coursedir}/ instead. So we're going to need some way of grabbing that stuff in the test runtime such that the json can be loaded from the correct place.

Copy link
Member

@oliverfoster oliverfoster Oct 6, 2023

Choose a reason for hiding this comment

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

sourcedir: 'src/',
outputdir: 'build/',
coursedir: 'course',

const outputdir = appendSlash(grunt.option('outputdir')) || exports.defaults.outputdir;
const cachepath = grunt.option('cachepath') || null;
const tempdir = outputdir + '.temp/';
const jsonext = grunt.option('jsonext') || exports.defaults.jsonext;
const coursedir = grunt.option('coursedir') || adaptJSON.coursedir || exports.defaults.coursedir;
let languageFolders = '';
if (grunt.option('languages') && grunt.option('languages').split(',').length > 1) {
languageFolders = '{' + grunt.option('languages') + '}';
} else {
languageFolders = grunt.option('languages');
}
// Selectively load the course.json ('outputdir' passed by server-build)
const configDir = grunt.option('outputdir') ? outputdir : sourcedir;
// add root path if necessary, and point to course/config.json
const configPath = path.join(path.resolve(root, configDir), coursedir, 'config.' + jsonext);

adapt_framework/test.js

Lines 69 to 72 in 0da0411

'node',
'./node_modules/grunt/bin/grunt',
'diff',
shouldUseOutputDir && `--outputdir=${outputDir}`

Copy link
Contributor

Choose a reason for hiding this comment

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

Cheers Oli
I understand wanting to account for configurable paths, but the default of the framework build is to the directories stated so we should be okay with these assumptions when it comes to the GHA PR testing
Or did you have something further in mind?

Copy link
Member

@oliverfoster oliverfoster Oct 6, 2023

Choose a reason for hiding this comment

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

A few people have mentioned they'd like to perform tests based upon the course content, effectively removing some Quality Assurance tasks. We should start from facilitating that point of view, even if we never get there. As the first test, this already relies on the defined course data but will currently fail if used on a course in say, an AAT environment, which specifies outputdir.

Copy link
Member

@oliverfoster oliverfoster Oct 6, 2023

Choose a reason for hiding this comment

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

The adapt-contrib-resources specific e2e tests should live in resources.

@joe-allen-89
Copy link
Contributor

New data loading needs to be implemented.

@cahirodoherty-learningpool
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants