-
Notifications
You must be signed in to change notification settings - Fork 15
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
Support 0-level assertion priorities on TestRun page #863
Changes from all commits
69245f8
619a5fe
0c099f2
51ae9a5
7c464db
bde9219
c7d57fa
5c4ec6a
7de158b
3cbe58a
635cf65
318b7f4
0d5d165
a26a572
62ecce2
45d3ed6
af9a5b0
3d1fbcb
942223d
bba8ad4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -405,6 +405,8 @@ class CommandsInput { | |
commands.push(command); | ||
commandsAndSettings.push({ | ||
command, | ||
commandId, | ||
presentationNumber: Number(presentationNumber), | ||
settings: _atMode, | ||
settingsText: assistiveTech.settings?.[_atMode]?.screenText || 'default mode active', | ||
settingsInstructions: assistiveTech.settings?.[_atMode]?.instructions || [ | ||
|
@@ -767,6 +769,9 @@ class BehaviorInput { | |
|
||
const { commandsAndSettings } = commandsInput.getCommands({ task: json.task }, mode); | ||
|
||
// Use to determine assertionExceptions | ||
const commandsInfo = json.commandsInfo?.[at.key]; | ||
|
||
return new BehaviorInput({ | ||
behavior: { | ||
description: titleInput.title(), | ||
|
@@ -778,7 +783,31 @@ class BehaviorInput { | |
setupScriptDescription: json.setup_script_description, | ||
setupTestPage: json.setupTestPage, | ||
assertionResponseQuestion: json.assertionResponseQuestion, | ||
commands: commandsAndSettings, | ||
commands: commandsAndSettings.map(cs => { | ||
const foundCommandInfo = commandsInfo?.find( | ||
c => | ||
cs.commandId === c.command && | ||
cs.presentationNumber === c.presentationNumber && | ||
cs.settings === c.settings | ||
); | ||
if (!foundCommandInfo || !foundCommandInfo.assertionExceptions) return cs; | ||
|
||
// Only works for v2 | ||
let assertionExceptions = json.output_assertions.map(each => each.assertionId); | ||
foundCommandInfo.assertionExceptions.split(' ').forEach(each => { | ||
let [priority, assertionId] = each.split(':'); | ||
const index = assertionExceptions.findIndex(each => each === assertionId); | ||
|
||
priority = Number(priority); | ||
assertionExceptions[index] = priority; | ||
Comment on lines
+801
to
+802
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Under the assumption that "fewer mutable bindings is better", these statements could be combined to allow swapping the earlier There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good |
||
}); | ||
// Preserve default priority or update with exception | ||
assertionExceptions = assertionExceptions.map((each, index) => | ||
isNaN(each) ? json.output_assertions[index].priority : each | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't the members of |
||
); | ||
|
||
return { ...cs, assertionExceptions }; | ||
}), | ||
assertions: (json.output_assertions ? json.output_assertions : []).map(assertion => { | ||
// Tuple array [ priorityNumber, assertionText ] | ||
if (Array.isArray(assertion)) { | ||
|
@@ -788,7 +817,7 @@ class BehaviorInput { | |
}; | ||
} | ||
|
||
// { assertionId, priority, assertionStatement, assertionPhrase, refIds, commandInfo, tokenizedAssertionStatements } | ||
// { assertionId, priority, assertionStatement, assertionPhrase, refIds, tokenizedAssertionStatements } | ||
return { | ||
priority: assertion.priority, | ||
assertion: | ||
|
@@ -815,7 +844,7 @@ class BehaviorInput { | |
* @param {UnexpectedInput} data.unexpectedInput | ||
*/ | ||
static fromCollectedTestCommandsKeysUnexpected( | ||
{ info, target, instructions, assertions }, | ||
{ info, target, instructions, assertions, commands }, | ||
{ commandsInput, keysInput, unexpectedInput } | ||
) { | ||
// v1:info.task, v2: info.testId | v1:target.mode, v2:target.at.settings | ||
|
@@ -834,7 +863,28 @@ class BehaviorInput { | |
specificUserInstruction: instructions.raw || instructions.instructions, | ||
setupScriptDescription: target.setupScript ? target.setupScript.description : '', | ||
setupTestPage: target.setupScript ? target.setupScript.name : undefined, | ||
commands: commandsAndSettings, | ||
commands: commandsAndSettings.map(cs => { | ||
const foundCommandInfo = commands.find( | ||
c => cs.commandId === c.id && cs.settings === c.settings | ||
); | ||
if (!foundCommandInfo || !foundCommandInfo.assertionExceptions) return cs; | ||
|
||
// Only works for v2 | ||
let assertionExceptions = assertions.map(each => each.assertionId); | ||
foundCommandInfo.assertionExceptions.forEach(each => { | ||
let { priority, assertionId } = each; | ||
const index = assertionExceptions.findIndex(each => each === assertionId); | ||
|
||
priority = Number(priority); | ||
assertionExceptions[index] = priority; | ||
}); | ||
// Preserve default priority or update with exception | ||
assertionExceptions = assertionExceptions.map((each, index) => | ||
isNaN(each) ? assertions[index].priority : each | ||
); | ||
|
||
return { ...cs, assertionExceptions }; | ||
}), | ||
assertions: assertions.map( | ||
({ priority, expectation, assertionStatement, tokenizedAssertionStatements }) => { | ||
let assertion = tokenizedAssertionStatements | ||
|
@@ -1187,6 +1237,7 @@ export class TestRunInputOutput { | |
description: command.settings, | ||
text: command.settingsText, | ||
instructions: command.settingsInstructions, | ||
assertionExceptions: command.assertionExceptions, | ||
}, | ||
atOutput: { | ||
highlightRequired: false, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,6 +123,7 @@ export class commandsAPI { | |
return { | ||
value, | ||
key, | ||
settings: mode, | ||
}; | ||
}) | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
'use strict'; | ||
|
||
const { regenerateResultsAndRecalculateHashes } = require('./utils'); | ||
/** @type {import('sequelize-cli').Migration} */ | ||
module.exports = { | ||
async up(queryInterface, Sequelize) { | ||
const updateV2TestsToIncludeEmptyAssertionExceptions = | ||
async transaction => { | ||
const testPlanVersions = await queryInterface.sequelize.query( | ||
`SELECT id, tests FROM "TestPlanVersion" WHERE metadata->>'testFormatVersion' = '2'`, | ||
{ | ||
type: Sequelize.QueryTypes.SELECT, | ||
transaction | ||
} | ||
); | ||
await Promise.all( | ||
testPlanVersions.map(async ({ id, tests }) => { | ||
const updatedTests = JSON.stringify( | ||
tests.map(test => { | ||
test.assertions = test.assertions.map( | ||
assertion => ({ | ||
...assertion, | ||
assertionExceptions: [] | ||
}) | ||
); | ||
return test; | ||
}) | ||
); | ||
await queryInterface.sequelize.query( | ||
`UPDATE "TestPlanVersion" SET tests = ? WHERE id = ?`, | ||
{ replacements: [updatedTests, id], transaction } | ||
); | ||
}) | ||
); | ||
}; | ||
|
||
return queryInterface.sequelize.transaction(async transaction => { | ||
await updateV2TestsToIncludeEmptyAssertionExceptions(transaction); | ||
|
||
// Recalculate the hashes | ||
await regenerateResultsAndRecalculateHashes( | ||
queryInterface, | ||
transaction | ||
); | ||
}); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array.prototype.indexOf
would be a bit more readable hereThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed