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

Update BDDRunner.cfc #150

Merged
merged 2 commits into from
Aug 16, 2023
Merged

Conversation

lhoess
Copy link
Contributor

@lhoess lhoess commented Aug 10, 2023

Description

The variable thisSuite isn't defined if the for loop in the try/catch is never reached before the error.
Found this would occur when the model threw an error.

Please note that all PRs must have tests attached to them

IMPORTANT: Please review the CONTRIBUTING.md file for detailed contributing guidelines.

Jira Issues

All PRs must have an accompanied Jira issue. Please make sure you created it and linked it here.

Bug Tracker: https://ortussolutions.atlassian.net/jira/software/c/projects/TESTBOX/issues

Type of change

  • Bug Fix

Checklist

  • New and existing unit tests pass locally with my changes

variable thisSuite isn't defined if the for loop is never reached.
@@ -153,7 +153,7 @@ component
e,
arguments.target,
arguments.testResults,
thisSuite
thisSuite?:""
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you change it to a ternary, there are still many issues with Adobe engines and elvis.

isNull( thisSuite ) ? {} : thisSuite

Changed to ternary operation per Luis.
@lhoess
Copy link
Contributor Author

lhoess commented Aug 11, 2023

Updated change to ternary per Luis.

@lhoess lhoess closed this Aug 11, 2023
@lmajano
Copy link
Contributor

lmajano commented Aug 15, 2023

hmm, why did you close it? @lhoess

@lhoess lhoess reopened this Aug 15, 2023
@lhoess
Copy link
Contributor Author

lhoess commented Aug 15, 2023

Because I'm not sure what I'm doing ;-) and couldn't see how to submit my new changes to the request.
I think I've reopened it now.

@lmajano lmajano marked this pull request as ready for review August 16, 2023 14:46
@lmajano
Copy link
Contributor

lmajano commented Aug 16, 2023

You did great!!!

@lmajano lmajano merged commit d899892 into Ortus-Solutions:development Aug 16, 2023
@lmajano lmajano mentioned this pull request Sep 13, 2023
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.

2 participants