Add tests for TilingSprite.getLocalBounds#4752
Conversation
ae8da01 to
9aeb6d2
Compare
ivanpopelyshev
left a comment
There was a problem hiding this comment.
Wait, WHAT? I thought we fixed it.
| this._bounds.minY = this._height * -this._anchor._y; | ||
| this._bounds.maxX = this._width * (1 - this._anchor._x); | ||
| this._bounds.maxY = this._height * (1 - this._anchor._x); | ||
| this._bounds.maxY = this._height * (1 - this._anchor._y); |
| this.tileSprite._anchor = { | ||
| _x: 3, | ||
| _y: 4, | ||
| }; |
There was a problem hiding this comment.
In general, I'm not really a fan how anti-pattern the code in your tests are. Why are you not doing this.tilingSprite.anchor.set(3, 4) here or setting width and height in the TilingSprite constructor in before? It's not good form to set private variables, similar to my comment in your last PR.
|
|
||
| after(function () | ||
| { | ||
| this.tileSprite.children = []; |
There was a problem hiding this comment.
Don't re-assign children. this.tileSprite.children.length = 0
|
|
||
| beforeEach(function () | ||
| { | ||
| sinon.stub(PIXI.Sprite.prototype, 'getLocalBounds'); |
There was a problem hiding this comment.
Why PIXI.Sprite.prototype and not TIlingSprite's prototype?
There was a problem hiding this comment.
Because return super.getLocalBounds.call(this, rect);, calls PIXI.Sprite.prototype.getLocalBounds, not PIXI.extras.TilingSprite.prototype.getLocalBounds.
For experiment, I changed sinon.stub(PIXI.Sprite.prototype, 'getLocalBounds'); to sinon.stub(PIXI.extras.TilingSprite.prototype, 'getLocalBounds'); and had got failed tests, as expected.
9aeb6d2 to
0789332
Compare
0789332 to
db9bb6c
Compare
| this.tileSprite._anchor = { | ||
| _x: 3, | ||
| _y: 4, | ||
| }; |
|
|
||
| beforeEach(function () | ||
| { | ||
| sinon.stub(PIXI.Sprite.prototype, 'getLocalBounds'); |
There was a problem hiding this comment.
Because return super.getLocalBounds.call(this, rect);, calls PIXI.Sprite.prototype.getLocalBounds, not PIXI.extras.TilingSprite.prototype.getLocalBounds.
For experiment, I changed sinon.stub(PIXI.Sprite.prototype, 'getLocalBounds'); to sinon.stub(PIXI.extras.TilingSprite.prototype, 'getLocalBounds'); and had got failed tests, as expected.
| before(function () | ||
| { | ||
| this.tileSprite = new PIXI.extras.TilingSprite(PIXI.Texture.EMPTY, 1, 2); | ||
| this.tileSprite.anchor.set(3, 4); |
There was a problem hiding this comment.
In my opinion, it's simplier to manage used properties (even private) directly, than to make test depending from another method like anchor.set(x, y). Because if it will be wrong, the target test will be wrong too, which looks more like integration testing, than unittesting.
But of course, here I will follow project guidelines.
| this._bounds.minY = this._height * -this._anchor._y; | ||
| this._bounds.maxX = this._width * (1 - this._anchor._x); | ||
| this._bounds.maxY = this._height * (1 - this._anchor._x); | ||
| this._bounds.maxY = this._height * (1 - this._anchor._y); |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
No description provided.