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

Fix unit tests #693

Merged
merged 13 commits into from Jun 22, 2020
Merged

Fix unit tests #693

merged 13 commits into from Jun 22, 2020

Conversation

alalamav
Copy link
Contributor

@alalamav alalamav commented Jun 12, 2020

  • Since migrating to webpack the unit tests have not been running. This is due to the fact that we create a single JS bundle, whereas jasmine expects individual JS spec files.
  • Fixes shadowbox and the manager unit testing by transpiling individual files via tsc (no webpack).
  • The manager web app tests for app.spec.ts are still not running because app.ts dependencies (notably polymer) target the browser and use ES6 imports, which is not compatible with jasmine.

@alalamav alalamav self-assigned this Jun 12, 2020
@@ -0,0 +1,11 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Is having a separate tsconfig here going to keep us from sharing code between server_manager and shadowbox? I thought we were trying to work towards that end state

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so - sharing code between workspaces should be orthogonal to the build configuration of each package.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also: you can't share code that isn't shareable. If code depends on the broswer environment, you can't run on Node. If code depends on the Node environment, you can't run on the browser. There are polyfills for some things, but that's limited.

@alalamav alalamav changed the title Fix shadowbox and manager Electron app unit tests Fix unit tests Jun 19, 2020
@alalamav alalamav marked this pull request as ready for review June 19, 2020 15:28
@alalamav alalamav requested a review from fortuna June 19, 2020 15:28
Copy link
Collaborator

@fortuna fortuna left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the tests.

I think we can submit this, with some tweaks to the scripts that I mentioned.
However, we can still consider using webpack. It's possible to make it take all .spec.ts files as entrypoints, and output them separately. Then all you need to do is call jasmine on them. But that could be on another PR.

src/server_manager/electron_app/test_action.sh Outdated Show resolved Hide resolved
src/server_manager/tsconfig.json Show resolved Hide resolved
src/shadowbox/test_action.sh Outdated Show resolved Hide resolved
@alalamav alalamav merged commit 2790202 into master Jun 22, 2020
@alalamav alalamav deleted the alalama-unit-tests branch June 22, 2020 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants