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 unit tests #283

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

atharvarajurkar
Copy link

No description provided.

Copy link
Owner

@rugk rugk left a comment

Choose a reason for hiding this comment

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

Hi @atharvarajurkar,
thanks for your first contribution to this project! 🎉 👍 🏅
I hope you'll like this project and enjoy hacking on it… 😃

Did you actually run these tests according to the contribution guidelines? (As these are unfortunately quite hard to run…) If so a screenshot of that would be nice.

But before you do the work, see my other comment, as I assume you can delete the lodash files again. We do not need to test stuff that is already tested.

Anyway, I hope this was a good practice and you've learned unit testing things.


import isObjectLike from "/common/modules/lodash/isObjectLike.js";

describe("common lodash: isObjectLike", function () {
Copy link
Owner

Choose a reason for hiding this comment

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

That's a very good intent, but I doubt we need to test lodash modules again. They are teted by lodash already and may/are in the future even considered to be dropped: https://github.com/TinyWebEx/AddonTemplate/issues/7

But sure this was a good test to do,

Same goes for the other files. BrowserCommunicationTypes is a thing I would keep, the other ones are kinda redundant.

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.

None yet

2 participants