#900 Add tests for events on Terminal #1646
Conversation
There was a problem hiding this comment.
Looks good! The only thing is that I think all the tests should be async with the done handler happening inside them. While some currently would happen synchronously, this will future proof us as we typically don't make a guarantee that these things will happen sync.
src/Terminal.test.ts
Outdated
| expect(event).to.be.an.instanceof(Object); | ||
| }); | ||
|
|
||
| term.keyDown(evKeyDown); |
There was a problem hiding this comment.
This test needs to be asynchronous and verify both events pass, as currently it will pass if key and keydown don't fire
src/Terminal.test.ts
Outdated
| expect(event).to.be.an.instanceof(Object); | ||
| }); | ||
|
|
||
| term.keyPress(evKeyPress); |
There was a problem hiding this comment.
This test needs to be asynchronous and verify both events pass, as currently it will pass if key and keypress don't fire
src/Terminal.test.ts
Outdated
| assert.equal(typeof data.start, 'number'); | ||
| assert.equal(typeof data.end, 'number'); | ||
| }); | ||
| term.refresh(0, term.rows - 1); |
There was a problem hiding this comment.
This test needs to be asynchronous and verify the event happens (add done to end of event listener)
There was a problem hiding this comment.
After additional investigation, I've found out that refresh is a dummy function.
Next thing is that the MockRenderer also does not implement any functionality.
There are two ways:
- Extend mock functionality (because we're unable to use real renderers due to lack of
windowobject, etc). - Skip implementation of this test completely or implement a dummy test.
Don't know which one is preferable. 馃槂
src/Terminal.test.ts
Outdated
| assert.equal(typeof data.cols, 'number'); | ||
| assert.equal(typeof data.rows, 'number'); | ||
| }); | ||
| term.resize(1, 1); |
src/Terminal.test.ts
Outdated
| term.on('scroll', (ydisp) => { | ||
| assert.equal(typeof ydisp, 'number'); | ||
| }); | ||
| term.scroll(); |
src/Terminal.test.ts
Outdated
| term.on('title', (title) => { | ||
| assert.equal(typeof title, 'string'); | ||
| }); | ||
| term.handleTitle('title'); |
|
List of questions I have right now.
Or, maybe, you can give me a hint to some other solution? Thanks in advance 馃槂 |
|
@peacefullatom sorry about the delay 馃槄
|
There was a problem hiding this comment.
@peacefullatom also I'm happy to merge this now if tests pass 馃槂
|
No problem! 馃槂 |
Here is my version of tests for events.
Honestly, don't know how to implement tests for 'blur' | 'focus' | 'linefeed' | 'selection', because they are emitted from outer components.
Please, let me know if I can add/improve something. 馃槂
Fixes #900