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

[test] ditch sinon module #5429

Open
akosyakov opened this issue Jun 12, 2019 · 2 comments
Open

[test] ditch sinon module #5429

akosyakov opened this issue Jun 12, 2019 · 2 comments
Labels
proposal feature proposals (potential future features) quality issues related to code and application quality test issues related to unit and api tests

Comments

@akosyakov
Copy link
Member

Sinon library allows to mock JS objects. I see a lot of tests misusing it by mocking internals and relying on timing and execution order of internals. Such tests hard to maintain since they don't allow to change implementations easily. Instead of developers should rely on public APIs and don't make assumptions about internal flow. The issue with Sinon that it provides capabilities to mock such things easily.

I suggest we remove it and use plain JS for object mocking, i.e. <MyService>{}. It will make hard to mock internal control flow and discourage it. Plus we will reduce our dep tree and force to review test for such misusage.

@akosyakov akosyakov added quality issues related to code and application quality proposal feature proposals (potential future features) test issues related to unit and api tests labels Jun 12, 2019
@vince-fugnitto
Copy link
Member

I'm fine with removing sinon :)

@akosyakov
Copy link
Member Author

It was pointed out at the dev meeting that we should define what is good test and look at it during PR review. Using sinon itself is not the reason for unstable tests.

I will document definition of good and bad tests with examples here: https://github.com/theia-ide/theia/blob/master/doc/Testing.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal feature proposals (potential future features) quality issues related to code and application quality test issues related to unit and api tests
Projects
None yet
Development

No branches or pull requests

2 participants