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

Make acceptance tests for comments more robust #12649

Merged
merged 2 commits into from
Nov 26, 2018

Conversation

danxuliu
Copy link
Member

How to test:

Result with this pull request:
All tests pass

Previous result:
open the comments for a different file failed due to the empty content message not having being shown yet when looking for it and not waiting for it to be shown, write a comment in a file right after writing a comment in another file failed due to Comment in Folder being shown in welcome.txt.

This last failure is actually a bug in the comments, but one that can not be reliably tested with the acceptance tests due to depending on how fast the server creates the new comment. Therefore, the scenario now waits for the comment to be added to the first file before continuing; this still verifies that changing to a different file does not cause the comments from the previous file to be shown in the current file (this was a different bug already fixed and due to which this test was added in the first place), although it no longer verifies that changing to a different file while the comment is being sent does not cause the comment from the previous file to be shown in the current file (subtle difference, but needed for the reliability of the tests ;-) ).

When the "Comments" tab is open the empty content element is always in
the DOM, although it is only shown once the message collection was
fetched and there were no messages. Due to this it is necessary to
explicitly wait for it to be shown instead of relying on the implicit
wait made to find the element; otherwise it would be found immediately
and if the collection was not fetched yet it would not be visible,
causing the test to fail.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
If the server is too slow, changing to a different file immediately
after sending a new comment but without waiting for the comment to be
shown for the original file could cause the new comment to be shown for
the current file instead.

This is, indeed, a bug in the comments. However, it is not possible to
test it reliably in the acceptance tests, as it depends on how fast the
server adds the message and how fast the client changes to a different
file; sometimes the test would fail and sometimes it would not.

Therefore, now it is waited for the comment to be added before changing
to another file, as in this case it can be reliably tested that changing
to a different file does not cause the comments from the previous file
to be shown in the current file (this was a different bug already fixed
and due to which this test was added in the first place).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu added the 3. to review Waiting for reviews label Nov 24, 2018
@danxuliu danxuliu added this to the Nextcloud 16 milestone Nov 24, 2018
@danxuliu
Copy link
Member Author

/backport to 15

@danxuliu
Copy link
Member Author

/backport to 14

@MorrisJobke MorrisJobke merged commit 2170027 into master Nov 26, 2018
@MorrisJobke MorrisJobke deleted the make-acceptance-tests-for-comments-more-robust branch November 26, 2018 10:23
@danxuliu
Copy link
Member Author

/backport to 15

@danxuliu
Copy link
Member Author

/backport to 14

@kesselb
Copy link
Contributor

kesselb commented Nov 27, 2018

/backport to stable15

@backportbot-nextcloud
Copy link

backport to stable15 in #12696

@kesselb
Copy link
Contributor

kesselb commented Nov 27, 2018

/backport to stable14

@backportbot-nextcloud
Copy link

backport to stable14 in #12697

@danxuliu
Copy link
Member Author

Well, this is embarrassing xD Thanks @danielkesselberg for the correct commands :-)

@kesselb
Copy link
Contributor

kesselb commented Nov 27, 2018

Hmm. I guess the bot should not react with 👍 when he does not understand the command 😕

@danxuliu
Copy link
Member Author

Hmm. I guess the bot should not react with 👍 when he does not understand the command 😕

I agree: nextcloud/backportbot#13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants