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

Add unmock function #212

Closed
wants to merge 16 commits into from
Closed

Add unmock function #212

wants to merge 16 commits into from

Conversation

Chemaclass
Copy link
Member

@Chemaclass Chemaclass commented Oct 30, 2023

📚 Description

Currently, you can mock a particular function with custom logic, but there is no way to rollback the mock for other tests. It would be useful to be able to remove/rollback the mocked behaviour.

🔖 Changes

  • Add unmock function

All mocks will be removed/rollback to their original behaviour after each test.
This means that the mock live scope is tied to the test scope

✅ To-do list

  • I updated the CHANGELOG.md to reflect the new feature or fix
  • I updated the documentation to reflect the changes

@Chemaclass Chemaclass added the enhancement New feature or request label Oct 30, 2023
@Chemaclass Chemaclass self-assigned this Oct 30, 2023
tests/unit/test_doubles_test.sh Outdated Show resolved Hide resolved
src/runner.sh Outdated
@@ -127,6 +127,7 @@ function runner::run_test() {
"$function_name" "$data" 2>&1 1>&3

runner::run_tear_down
runner::clean_mocks
Copy link
Member

Choose a reason for hiding this comment

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

Where are the test?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added them to tests/acceptance/mock_test.sh

Copy link
Member

Choose a reason for hiding this comment

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

we are also missing unit test of the function, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you give me a hint how would you write unit tests for runner::clear_mocks()?

src/runner.sh Outdated
@@ -189,6 +190,12 @@ function runner::run_tear_down() {
helper::execute_function_if_exists 'tear_down'
}

function runner::clean_mocks() {
Copy link
Member

@khru khru Oct 30, 2023

Choose a reason for hiding this comment

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

Where are the test?

@khru khru requested a review from JesusValeraDev October 30, 2023 15:58
@Chemaclass Chemaclass requested a review from khru October 30, 2023 16:07
src/runner.sh Outdated
@@ -127,6 +127,7 @@ function runner::run_test() {
"$function_name" "$data" 2>&1 1>&3

runner::run_tear_down
runner::clean_mocks
Copy link
Member

Choose a reason for hiding this comment

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

we are also missing unit test of the function, no?

src/runner.sh Outdated Show resolved Hide resolved
function test_runner_clean_mocks_2() {
assert_is_not_mock ls
assert_is_not_mock ps
}
Copy link
Member

Choose a reason for hiding this comment

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

These test are not acceptance test, and the behavior you are trying to test it's not clear

tests/unit/test_doubles_test.sh Show resolved Hide resolved
@antonio-gg-dev
Copy link
Member

@Chemaclass this is more complex than we anticipated, do you mind if we leave it in draft until we can get together? Besides, I think it's more priority to refactor the acceptance tests and finish the snapshots.

@Chemaclass Chemaclass marked this pull request as draft October 31, 2023 10:06
@Chemaclass
Copy link
Member Author

Close because inactivity. I will open it again to seek for new feedback

@Chemaclass Chemaclass closed this Dec 10, 2023
@Chemaclass Chemaclass deleted the feat/unmock branch December 10, 2023 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants