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

Create the "equalMarkup" assertion #9668

Closed
pomek opened this issue May 11, 2021 · 4 comments · Fixed by ckeditor/ckeditor5-dev#733 or #10871
Closed

Create the "equalMarkup" assertion #9668

pomek opened this issue May 11, 2021 · 4 comments · Fixed by ckeditor/ckeditor5-dev#733 or #10871
Assignees
Labels
intro Good first ticket. package:dev squad:platform Issue to be handled by the Platform team. type:improvement This issue reports a possible enhancement of an existing feature.

Comments

@pomek
Copy link
Member

pomek commented May 11, 2021

📝 Provide a description of the improvement

We already have the assertEqualMarkup() util for comparing markups.

/**
* An assertion util test whether two given strings containing markup language are equal.
* Unlike `expect().to.equal()` form Chai assertion library, this util formats the markup before showing a diff.
*
* This util can be used to test HTML strings and string containing serialized model.
*
* // Will throw an error that is handled as assertion error by Chai.
* assertEqualMarkup(
* '<paragraph><$text foo="bar">baz</$text></paragraph>',
* '<paragraph><$text foo="bar">baaz</$text></paragraph>',
* );
*
* @param {String} actual An actual string.
* @param {String} expected An expected string.
* @param {String} [message="Expected two markup strings to be equal"] Optional error message.
*/
export function assertEqualMarkup( actual, expected, message = 'Expected markup strings to be equal' ) {
if ( actual != expected ) {
throw new AssertionError( message, {
actual: formatMarkup( actual ),
expected: formatMarkup( expected ),
showDiff: true
} );
}
}

However, many of us may do not know about the helper. Also, it requires adding additional imports.

We could add a new function to chai (https://www.chaijs.com/guide/helpers/#adding-methods) that will introduce the new assertion. The goal is to have something like this:

expect( editor.getData() ).to.equalMarkup( /* ... */ );

In details, the function will use the assertEqualMarkup() helper.

It would be nice to add the new helper to existing bindings to have the new assertion in IDE autocomplete list.


If you'd like to see, this improvement implemented, add a 👍 reaction to this post.

@pomek pomek added type:improvement This issue reports a possible enhancement of an existing feature. intro Good first ticket. squad:platform Issue to be handled by the Platform team. package:dev labels May 11, 2021
@pomek pomek added this to the nice-to-have milestone May 11, 2021
@pomek pomek removed the squad:platform Issue to be handled by the Platform team. label Oct 5, 2021
@Reinmar Reinmar added squad:platform Issue to be handled by the Platform team. and removed squad:platform Issue to be handled by the Platform team. labels Oct 28, 2021
@pomek
Copy link
Member Author

pomek commented Nov 8, 2021

Scope:

  • Create a new mocha assertion.
  • Replace existing equalMarkup functions with the created assertion.

@pomek pomek modified the milestones: nice-to-have, iteration 49 Nov 8, 2021
@przemyslaw-zan przemyslaw-zan self-assigned this Nov 16, 2021
@przemyslaw-zan
Copy link
Member

Normaly, you'd have to import this new assertion to use it within tests. However, there is a way of avoiding that - any new chai assertions can be imported in this file - after that, they are available in all tests.

attribute.js is an example of already existing custom chai assertion that has to be imported manually whenever needed. Do we want to move that to ckeditor5-dev-tests and include it in automatic import for all tests, so that there is no need to import that whenever needed? 

There might be other custom chai assertions that i'm not aware of, that also could be moved there.

@pomek
Copy link
Member Author

pomek commented Nov 18, 2021

attribute.js is an example of already existing custom chai assertion that has to be imported manually whenever needed. Do we want to move that to ckeditor5-dev-tests and include it in automatic import for all tests, so that there is no need to import that whenever needed? 

Wow. I wasn't aware of this. Yes, let's move all custom assertions to ckeditor5-dev-tests and enable using them in any test without importing it directly.

@przemyslaw-zan
Copy link
Member

It would be nice to add the new helper to existing bindings to have the new assertion in IDE autocomplete list.

We ended up skipping that part, so perhaps it could be extracted to a followup?

pomek added a commit to ckeditor/ckeditor5-dev that referenced this issue Nov 24, 2021
Feature (tests): Created the `equalMarkup` and `attribute` chai assertions. They are loaded automatically when running tests. Closes ckeditor/ckeditor5#9668.
pomek added a commit that referenced this issue Nov 24, 2021
Other: Removed the `assertEqualMarkup` function and the `attribute` assertion as they have been moved to the testing environment. Closes #9668.

Tests: Updated all files using `attribute` and `equalMarkup` assertions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
intro Good first ticket. package:dev squad:platform Issue to be handled by the Platform team. type:improvement This issue reports a possible enhancement of an existing feature.
Projects
None yet
3 participants