-
Notifications
You must be signed in to change notification settings - Fork 47
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
GL-2753 : CS Wizard updated node version select values #1767
Conversation
Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/1767/acli.phar
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1767 +/- ##
============================================
+ Coverage 91.92% 91.99% +0.07%
Complexity 1818 1818
============================================
Files 121 121
Lines 6834 6834
============================================
+ Hits 6282 6287 +5
+ Misses 552 547 -5 ☔ View full report in Codecov by Sentry. |
@@ -65,6 +65,8 @@ public function providerTestCommand(): array | |||
[$this->getMockedGitLabProject($this->gitLabProjectId)], | |||
// Inputs. | |||
[ | |||
0, |
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.
The test cases are all providing invalid input, and since there are practically no assertions they were overlooked.
This fixes the inputs to at least be valid, but I'm not sure if the test cases are actually testing what they're supposed to...
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.
That is what I failed to understand, even when the inputs were wrong the output should not have contained the success messages which was asserted in the last.
I will probably keep this one to be worked upon in future.
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.
Yeah, I think this method only returns one line. Ideally you'd be checking the entire output:
$output_strings = $this->getOutputStrings();
@danepowell - would you be able to release this change? GL-2653 depends on this update. |
Motivation
The current values provided in NODE_VERSION need to be update as for better onboarding flow to user on Node Project. Also the default value for the select needed to be changed.
Testing steps
Following steps can be taken to verify the changes:
acli cs:wizard
.