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

6574: Use TableEditing in tests. #7410

Merged
merged 32 commits into from
Jun 10, 2020
Merged

6574: Use TableEditing in tests. #7410

merged 32 commits into from
Jun 10, 2020

Conversation

jodator
Copy link
Contributor

@jodator jodator commented Jun 9, 2020

Suggested merge commit message (convention)

Other (table): Use real schema and conversion definitions in table tests. Closes #6574.

Other (table): Removed options.asWidget from most of the table converters which are never run in data pipeline.


Additional information

This PR make a total cleanup for table tests:

  1. Use TableEditng for all the tests - no more schema/conversion stubbing.
  2. Only downcastInsertTable() is tested for data pipeline (the data pipeline is static conversion - no need to tests changing heading rows or inserting table cells there)
  3. As of 2. the asWidget option make no sense in most of the converters.

@jodator jodator requested a review from niegowski June 9, 2020 10:59
Copy link
Contributor

@niegowski niegowski left a comment

Choose a reason for hiding this comment

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

I would add tests to make sure that some conversions are not used in data pipeline.

} );

describe( 'downcastInsertRow()', () => {
// The insert row downcast conversion is not executed in data pipeline.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add test to make sure that row conversion is not used in data pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not able to think of such test :) It would be very synthetic. The data pipeline conversion runs on whole, static content it do not convert "dynamic" actions (adding/removing stuff, changing attributes, etc).

@jodator jodator requested a review from niegowski June 10, 2020 07:52
root = doc.getRoot( 'main' );
view = editor.editing.view;

markerConversion( editor.conversion, 'marker' );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No expclicit test for classes as an array. /cc @niegowski

@jodator
Copy link
Contributor Author

jodator commented Jun 10, 2020

@niegowski the tests from #7391 were also fixed.

@niegowski niegowski merged commit b127f41 into master Jun 10, 2020
@niegowski niegowski deleted the i/6574 branch June 10, 2020 13:05
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.

The table tests are wrong
2 participants