[RFR] automate MTA-382--assess application with multiple questionnaires #828
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
initial commit
midays
changed the title
nachandr
suggested changes
Nov 22, 2023
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Show resolved
Hide resolved
sshveta
requested changes
Nov 22, 2023
cypress/e2e/models/migration/applicationinventory/assessment.ts
Outdated
Show resolved
Hide resolved
cypress/e2e/models/migration/applicationinventory/assessment.ts
Outdated
Show resolved
Hide resolved
cypress/e2e/models/migration/applicationinventory/assessment.ts
Outdated
Show resolved
Hide resolved
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Outdated
Show resolved
Hide resolved
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Outdated
Show resolved
Hide resolved
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Outdated
Show resolved
Hide resolved
sshveta
reviewed
Nov 23, 2023
cypress/e2e/models/migration/applicationinventory/assessment.ts
Outdated
Show resolved
Hide resolved
sshveta
requested changes
Nov 23, 2023
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Show resolved
Hide resolved
final push
npm run format
ibragins
reviewed
Nov 26, 2023
cypress/e2e/models/migration/applicationinventory/assessment.ts
Outdated
Show resolved
Hide resolved
midays
changed the title
updated timeout to 15 seconds
changed 2000 to 2 * SEC
|
Please attach a jenkins run @midays |
nachandr
suggested changes
Nov 27, 2023
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Outdated
Show resolved
Hide resolved
added assessment validation
abrugaro
requested changes
Nov 27, 2023
Comment on lines
151
to
181
| } else if (risk === "medium") { | ||
| cy.wrap($question) | ||
| .children() | ||
| .find("div.pf-v5-l-split__item") | ||
| .then(($questionLine) => { | ||
| /* These 3 questions generate high risk with mean options, | ||
| hence to keep risk to medium, select last options for these set of specific questions */ | ||
| if ( | ||
| $questionLine.text() === | ||
| "Does the application have legal and/or licensing requirements?" || | ||
| $questionLine.text() === | ||
| "Does the application require specific hardware?" || | ||
| $questionLine.text() === | ||
| "How is the application clustering managed?" | ||
| ) { | ||
| optionToSelect = totalOptions - 1; | ||
| } else { | ||
| optionToSelect = Math.floor(totalOptions / 2); | ||
| } | ||
| this.clickRadioOption($question, optionToSelect); | ||
| }); | ||
| } else { | ||
| optionToSelect = 1; | ||
| this.clickRadioOption($question, optionToSelect); | ||
| } | ||
| }); | ||
| if (i === 4) { | ||
| clickJs(commonView.nextButton); | ||
| } else { | ||
| clickJs(commonView.nextButton); | ||
| } |
There was a problem hiding this comment.
This method is too complex right now, some considerations:
lowandhighrisk conditions are almost equal, so they can be combined with something like:
if(risk === "medium") {
...
} else {
this.clickRadioOption($question, risk === "high" ? 1 : totalOptions - 1);
}- Those 3 special questions are making the if condition too long, and in the future some questions may change, be added or removed.
In order to make the code more maintainable, an idea could be to save the questions in an array at the start of the method and use includes instead of doing multipleConditions:
const specialHighRiskQuestions = [
"Does the application have legal and/or licensing requirements?",
"Does the application require specific hardware?",
"How is the application clustering managed?"
];
...
...
if(specialHighRiskQuestions.includes($questionLine.text())) {
...
}- The last if-else looks identical to me:
if (i === 4) {
clickJs(commonView.nextButton);
} else {
clickJs(commonView.nextButton);
}There was a problem hiding this comment.
There was a problem hiding this comment.
At least point 3 should be fixed before merging.
There was a problem hiding this comment.
all of the points you mentioned are change worthy, but I dind't write any of them, I just wrapped it in a cy tag, and I don't intend to change anything in it for now, so I opened a ticket for it, and I'll work on it in the next PR
There was a problem hiding this comment.
@midays as discussed , implement point 3 . refactoring of answering questionnaire can be done in a separate PR . It will need thorough testing . Please open a card for it and assign it to me .
For this PR attach a jenkins run with this test
There was a problem hiding this comment.
@sshveta impletmented change, jenkins run passed: https://main-jenkins-csb-migrationqe.apps.ocp-c1.prod.psi.redhat.com/job/mta/job/mta-ui-tests-runner/1343/console
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Outdated
Show resolved
Hide resolved
cypress/e2e/tests/migration/applicationinventory/assessment/assess_review.test.ts
Outdated
Show resolved
Hide resolved
changed timeout to 30 seconds
abrugaro
reviewed
Nov 27, 2023
changed undefined to null
reload the page after opening the application page
enable the questioneer only when needed
import in TC
changed execution order
remove if statement
checkout cypress
remove condition
sshveta
reviewed
Nov 27, 2023
|
|
||
| const stakeholdersList: Array<Stakeholders> = []; | ||
| const stakeholdersNameList: Array<string> = []; | ||
| const fileName = "Legacy Pathfinder"; | ||
| const yamlFilePath = "questionnaire_import/cloud-native.yaml"; |
There was a problem hiding this comment.
Please rename this to "yamlFile"
rename file
sshveta
approved these changes
Nov 27, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: mohamed idays [email protected]