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 tests not building #14

Merged
merged 1 commit into from
Nov 8, 2018
Merged

Conversation

TheKnarf
Copy link
Contributor

@TheKnarf TheKnarf commented Nov 8, 2018

Running npm run build_tests did not initially work for me, these changes fixes that.
(Hopefully I didn't break any of the tests).

Perhaps we could setup Travis CI (or some other CI) to build and run tests to ensure that new code don't break anything?

@TheKnarf TheKnarf mentioned this pull request Nov 8, 2018
@TheKnarf
Copy link
Contributor Author

TheKnarf commented Nov 8, 2018

While I can now build the legacy tests I'm still not sure how to run Jsunit?

@johanneswilm
Copy link
Contributor

@TheKnarf I have been running jsunit in a browser and the test running worked for me during the upgrade from the pre-ES2015 code, which was quite useful as I could use it to check that everything was constantly running. Have you tried running them in a browser?

I agree on setting up Travis (or similar).

As for this patch - this doesn't look quite right to me (it looks a bit pre-ES2015 with the IIFEs - can you not just rename the second variable you declare with the same name or just leave out the let with the second declaration?). But I'll let you continue with this and then maybe we can clean this up together once you are done translating all the tests. Agreed?

@johanneswilm johanneswilm merged commit 01aeb54 into DesignLiquido:master Nov 8, 2018
@johanneswilm
Copy link
Contributor

@TheKnarf I have merged your changes and removed the IIFEs in a way that is working at least for me. Please let me know if it is not working for you.

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.

2 participants