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

Added Trashcan Tests #2568

Merged
merged 4 commits into from
Jun 13, 2019
Merged

Conversation

BeksOmega
Copy link
Collaborator

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide

The details

Resolves

N/A

Proposed Changes

Adds tests to documenta trashcan behavior.

Reason for Changes

To keep Beka from breaking things.

Test Coverage

This is tests silly!

Additional Information

The tests document the current behavior of the trashcan, but I don't think all of that behavior is good. I left TODO's in places where I think behavior should be changed.

Also this might not cover everything that needs to be tested. If you have suggestions I'd love to add more tests.

Copy link
Collaborator

@rachel-fenichel rachel-fenichel left a comment

Choose a reason for hiding this comment

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

Create an issue for those TODOs and tag it in the comment as TODO (#issueNumber) so we don't completely lose track of them.

I see what you mean about the behaviour being a little wrong. The alternative would be to do much more parsing, on arbitrarily large chunks of xml, so I vote we leave it as a low-priority issue.


suite("Events", function() {
test("Delete", function() {
sendDeleteEvent(
Copy link
Collaborator

Choose a reason for hiding this comment

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

sendDeleteEvent can add the <xml> and </xml> tags to remove duplication.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

@BeksOmega
Copy link
Collaborator Author

Create an issue for those TODOs and tag it in the comment as TODO (#issueNumber) so we don't completely lose track of them.

Done. When I was creating the issue I noticed that the problems with the disabled editable and movable attributes do not actually occure, because a block will never serialize to disabled="false" when deleted, so I removed those test cases.

@rachel-fenichel rachel-fenichel merged commit 2aa05c6 into google:develop Jun 13, 2019
@BeksOmega BeksOmega deleted the tests/Trashcan branch July 1, 2019 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants