Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix FindReplace Integration tests #12440

Merged
merged 1 commit into from
Jun 4, 2016
Merged

Fix FindReplace Integration tests #12440

merged 1 commit into from
Jun 4, 2016

Conversation

marcelgerber
Copy link
Contributor

Turns out we never adapted our tests for the altered behaviour introduced in #11869.
I tried my best to preserve the test cases.

cc @abose

@petetnt
Copy link
Collaborator

petetnt commented May 20, 2016

All tests pass, changes look fine, LGTM

@ficristo
Copy link
Collaborator

It seems to me that the order of tests will matter now, so if we will start to shuffle around the test code we could start to see some failures.
I wonder if we could do something more explicit about it...

@redmunds
Copy link
Contributor

@ficristo You're referring to the "remember the last search behavior", correct? At the very least, each test should store previous value (in beforeEach), then restore it after the test (in afterEach). A better approach would be to mock the mechanism that stores the last search, and then create/destroy a new mock for each test.

@ficristo
Copy link
Collaborator

@redmunds yes.
Maybe for now we can commit this and file a new issue about this thing: what do you all think?

@abose abose merged commit 0e606b6 into master Jun 4, 2016
@abose abose deleted the marcel/fix-find-tests branch June 4, 2016 12:34
@abose
Copy link
Contributor

abose commented Jun 4, 2016

Oops, Missed this test failure. Thanks @marcelgerber
The order of the tests will come into picture when the test cases are modified, in which case the author who writes the test will mostly become aware of this quirk. As this will put the test suite back to working state, Merging the fix for now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants