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

Added widget template with unit tests #112

Merged
merged 4 commits into from
Jun 17, 2021
Merged

Added widget template with unit tests #112

merged 4 commits into from
Jun 17, 2021

Conversation

JonasHelming
Copy link
Contributor

fixed #103

Adds an extended version of the widget template including unit tests.

How to test:

  • Create the template "Widget with Unit Tests"
  • Run tests with "yarn test"

Contributed on behalf of STMicroelectronics

Signed-off-by: Jonas Helming jhelming@eclipsesource.com

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonasHelming I'd be fine with adding the unit-tests directly in the widget template rather than splitting it up into two templates. Any reason we want to do the split?

@JonasHelming
Copy link
Contributor Author

Good question! Only reason would be separation of concerns and keep the plain widget template simpler. So you would prefer to merge them?

@JonasHelming
Copy link
Contributor Author

Please note, that I will contribute a template for Ui tests as well

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Jun 8, 2021

@JonasHelming I think merging them where we have the widget extension with unit-tests already available would be fine in my opinion. I would not use jest however as it is quite heavy, perhaps we should use what we have in the main repo (mocha).

For comparison, I believe extensions generated with yo code (vscode) also include unit-tests by default.

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Jun 8, 2021

I don't have a strong opinion of using mocha (or another testing framework) vs jest, so I'll leave the decision up to you :)

fixed #103

Contributed on behalf of STMicroelectronics

Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
@JonasHelming
Copy link
Contributor Author

merged!

src/app/index.ts Show resolved Hide resolved
src/app/index.ts Outdated Show resolved Hide resolved
src/app/index.ts Outdated Show resolved Hide resolved
templates/widget/__tests__/widget.test.ts Outdated Show resolved Hide resolved
templates/widget/__tests__/widget.test.ts Outdated Show resolved Hide resolved
templates/widget/__tests__/widget.test.ts Outdated Show resolved Hide resolved
templates/widget/__tests__/widget.test.ts Outdated Show resolved Hide resolved
templates/widget/__tests__/widget.test.ts Outdated Show resolved Hide resolved
templates/widget/__tests__/widget.test.ts Outdated Show resolved Hide resolved
templates/widget/configs/jest.config.ts Outdated Show resolved Hide resolved
Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My last minor comment is the name within the describe('').

templates/widget/widget.test.ts Outdated Show resolved Hide resolved
Co-authored-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JonasHelming the changes look good to me, can you please use the squash and merge option when merging so we can have a clean history?

@JonasHelming JonasHelming merged commit f275d56 into master Jun 17, 2021
@JonasHelming
Copy link
Contributor Author

Sure, thank you very much for the review, Vince!

@vince-fugnitto vince-fugnitto deleted the GH-103 branch June 17, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add unit tests to templates
2 participants