webcompat / webcompat-reporter-extensions

Browser extensions to help report site compatibilty issues.
26 stars 21 forks source link

Fixes #90. Unit test for isReportableURL #111

Closed OrionStar25 closed 6 years ago

OrionStar25 commented 6 years ago

r? @miketaylr

Is this what you mean by creating a separate module?

Write a unit test that imports it

You mean check if it has been implemented or not? Can you help me out a little here? I read abut intern testing a bit.

miketaylr commented 6 years ago

Is this what you mean by creating a separate module?

Almost! You've pulled it out into its own function, which is a great first step. Now you need to turn that into an es6 module. Check out the following:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/export

(or do some google searches, there's a lot of good articles out there).

here's an example of a module that exports the initAddon function:

https://github.com/webcompat/webcompat-reporter-extensions/blob/master/shared/base.js#L66

And where it gets imported:

https://github.com/webcompat/webcompat-reporter-extensions/blob/master/firefox/addon.js#L5

Once you have that working (putting isReportableURL into its own module and file, and importing that into base.js so it can continue to be used), you need to create a unit tests where that module method is imported, and some tests are written around that.

These docs will help you there:

https://theintern.io/docs.html#Intern/4/docs/docs%2Fwriting_tests.md/unit-tests https://theintern.io/docs.html#Intern/4/docs/docs%2Fwriting_tests.md/object

You can also take a look at the existing unit tests in the tree.

I'm on PTO until Thursday, but I'll log back in tomorrow night if you have any follow up questions!

OrionStar25 commented 6 years ago

r? @miketaylr Should I be writing tests in test-manifest.js ? Also, the test I would write would be : If there is a url, isReportableUrl should return a true(?)

OrionStar25 commented 6 years ago

r? @miketaylr I'm kind of stuck. Could you tell me what else needs to be done? I do need to include the module in the test file too, but do I import it or make a const of it? Also, I'm not quite sure how that works. Also, heavily sorry for this delay.

miketaylr commented 6 years ago

I do need to include the module in the test file too, but do I import it or make a const of it?

Yeah, this is the tricky part. You need to import it into the test file, but importing an ES6 module into the Node/Intern environment isn't super straightforward right now.

The following two links should provide some good direction!

https://github.com/standard-things/esm https://github.com/theintern/intern/issues/626

miketaylr commented 6 years ago

@OrionStar25 would you like to keep working on this, or should we close it, in case other contributors want to pick it up? Thanks!

OrionStar25 commented 6 years ago

@miketaylr you can close it, I can't seem to figure it out right now.

miketaylr commented 6 years ago

Thanks @OrionStar25.