Skip to content

[stable9] Backport flat reshare unit tests#26927

Merged
PVince81 merged 3 commits intostable9from
stable9-backport-tests-flatreshare
Jan 12, 2017
Merged

[stable9] Backport flat reshare unit tests#26927
PVince81 merged 3 commits intostable9from
stable9-backport-tests-flatreshare

Conversation

@PVince81
Copy link
Contributor

Since stable9 already has flat reshares, the unit tests need to also use the new sharing API when creating shares to make sure we're testing the correct results.

Also this is required by #26912 which adds a performance fix that uses a shortcut that limits recursion based on the idea that recursion is not needed any more due to flat reshares.

Required for #26912

Please review @SergioBertolinSG @jvillafanez @DeepDiver1975 @VicDeo

@mention-bot
Copy link

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975 to be a potential reviewer.

@jvillafanez
Copy link
Member

In general, I've seen several tests that have several calls to OCS. If the tests are about OCS, then only one call to OCS should be done unless the code is explicitly testing other thing (for example, that you get the same data you've created before).

Using the share manager might be enough to get the required information, for example to check that the share is already there and to retrieve it, specially if we don't want to deal with direct DB access.

@PVince81
Copy link
Contributor Author

@jvillafanez thanks for the review. Please note that this is only a backport of tests that already exist on 9.1 and master. I'm not willing to make any big changes there in the context of a backport.

We could of course improve the tests in the future on master if you think they are not suitable, but I don't think this has high priority right now.

@jvillafanez
Copy link
Member

Ok, let's leave it as technical debt 👍

@PVince81 PVince81 merged commit ed5e8e2 into stable9 Jan 12, 2017
@PVince81 PVince81 deleted the stable9-backport-tests-flatreshare branch January 12, 2017 13:49
@lock
Copy link

lock bot commented Aug 4, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants