refactor(compileSpec): make tests consistent #15141
Conversation
| { | ||
| 'no root element': 'dada', | ||
| 'multiple root elements': '<div></div><div></div>' | ||
| }, function($prop) { |
There was a problem hiding this comment.
Calling this $prop too is confusing (since it is not the same $prop as in the description).
Maybe template or something.
| expect(function() { | ||
| $compile('<p multi-root-elem></p>'); | ||
| }).toThrowMinErr('$compile', 'tplrt', 'Template for directive \'multiRootElem\' must have exactly one root element. '); | ||
| inject(function($compile, $templateCache, $rootScope) { |
| 'comments + whitespace': ' <!-- oh hi --> <div>Hello World!</div> <!-- oh hi -->\n' | ||
| }, function($prop) { | ||
|
|
||
| inject(function($compile, $templateCache, $rootScope) { |
| var element; | ||
| expect(function() { | ||
| element = $compile('<p template></p>')($rootScope); | ||
| }).not.toThrowMinErr('$compile', 'tplrt', 'Template for directive \'template\' must have exactly one root element.'); |
There was a problem hiding this comment.
.not.toThrow() is preferrable, since right now a small change in the message (or a typo) could render the test useless.
There was a problem hiding this comment.
Ok, but I think we should match against the error key (tplrt), because that is unlikely to change and we want to make sure that exact error is thrown.
There was a problem hiding this comment.
I am talking about negative assertions only (i.e. with .not). Unless there is a reason (e.g. there is another error thrown that we can't prevent), I prefer to have as little info as possible (in order to make the assertion as strong as possible).
For example: .not.toThrow() is stronger than .not.toThrowMinErr('$compile') or .not.toThrowMinErr('$compile', 'tplrt').
| @@ -2075,55 +2052,53 @@ describe('$compile', function() { | |||
| } | |||
| )); | |||
|
|
|||
| describe('replace and not exactly one root element', function() { | |||
There was a problem hiding this comment.
Does it have to be the same description as above? What is the difference?
There was a problem hiding this comment.
The one above is for template, this one is for templateUrl
|
Can you check / approve this one @gkalpak ? |
Narretz
force-pushed
the
refactor-compile-tests
branch
from
cce6662 to
18d772b
Compare
Narretz
force-pushed
the
refactor-compile-tests
branch
from
18d772b to
1017b20
Compare
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
refactor
What is the current behavior? (You can also link to an open issue here)
Tests are duplicated and have non-optimal expects
What is the new behavior (if this is a feature change)?
Tests use they and have been moved to one place