Skip to content

Improved shell usage#6387

Merged
Naktibalda merged 7 commits into5.0from
shell-improv
Mar 20, 2022
Merged

Improved shell usage#6387
Naktibalda merged 7 commits into5.0from
shell-improv

Conversation

@DavertMik
Copy link
Copy Markdown
Member

@DavertMik DavertMik commented Mar 14, 2022 •
edited
Loading

More pause options

  • all shell settings moved to Codecept\Lib\PauseShell
  • codecept_pause() function
  • added $this->pause() for unit tests
  • all pause commands to receive $vars as array:
codecept_pause(['user' => $user]);
$I->pause(['pageObject' => $pageObject]);

TODO: Documentation!

@DavertMik DavertMik requested review from Naktibalda and TavoNiievez and removed request for Naktibalda March 14, 2022 11:01
Naktibalda
Naktibalda reviewed Mar 14, 2022
View reviewed changes
Comment thread src/Codeception/Test/Unit.php Outdated
Naktibalda
Naktibalda requested changes Mar 14, 2022
View reviewed changes
Comment thread functions.php
Comment thread src/Codeception/Test/Unit.php Outdated
TavoNiievez
TavoNiievez reviewed Mar 14, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@TavoNiievez TavoNiievez left a comment

Choose a reason for hiding this comment

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

I don't think the behavior of this new $vars parameter is intuitive. Please mention the documentation PR when you have it.
On the other hand, will there be tests for this new feature?

@DavertMik
Copy link
Copy Markdown
Member Author

DavertMik commented Mar 14, 2022 •
edited
Loading

I don't think the behavior of this new $vars parameter is intuitive. Please mention the documentation PR when you have it.

Unfortunately, documentation is not with the source code now, so I didn't make that PR yet.

On the other hand, will there be tests for this new feature?

I think It can be tested in non-interactive mode. But not sure how.

@DavertMik
Copy link
Copy Markdown
Member Author

DavertMik commented Mar 14, 2022

Updated PR so codecept_pause would also pick up $this context

@TavoNiievez sorry, I won't be able to write tests because of lack of time
If you think you will be able to write a test for this using non-interactive mode please do it. Thanks!

Naktibalda
Naktibalda reviewed Mar 15, 2022
View reviewed changes
Comment thread src/Codeception/Util/Debug.php Outdated
@DavertMik DavertMik requested a review from Naktibalda March 19, 2022 08:43
@DavertMik
Copy link
Copy Markdown
Member Author

DavertMik commented Mar 19, 2022

@Naktibalda I updated the behavior, please merge if you are ok with this

Naktibalda
Naktibalda reviewed Mar 19, 2022
View reviewed changes
Comment thread src/Codeception/Util/Debug.php Outdated
$backtrace = debug_backtrace(DEBUG_BACKTRACE_PROVIDE_OBJECT, 3);
foreach ($backtrace as $backtraceStep) {
$class = $backtraceStep['class'] ?? null;
$fn = $backtraceStep['class'] ?? null;
Copy link
Copy Markdown
Member

@Naktibalda Naktibalda Mar 19, 2022

Choose a reason for hiding this comment

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

This line should be using $backtraceStep['function'], right?

Copy link
Copy Markdown
Member Author

@DavertMik DavertMik Mar 19, 2022

Choose a reason for hiding this comment

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

Ups, right!

DavertMik
DavertMik commented Mar 19, 2022
View reviewed changes
Comment thread src/Codeception/Util/Debug.php Outdated
Naktibalda
Naktibalda reviewed Mar 19, 2022
View reviewed changes
Comment thread src/Codeception/Test/Unit.php
@DavertMik DavertMik requested a review from Naktibalda March 19, 2022 23:35
Naktibalda
Naktibalda approved these changes Mar 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@Naktibalda Naktibalda left a comment

Choose a reason for hiding this comment

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

Looks good to me

@Naktibalda Naktibalda merged commit e25dff1 into 5.0 Mar 20, 2022
@Naktibalda Naktibalda deleted the shell-improv branch March 20, 2022 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Naktibalda Naktibalda Naktibalda approved these changes

@TavoNiievez TavoNiievez TavoNiievez approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DavertMik @Naktibalda @TavoNiievez