Closed PaulieScanlon closed 3 years ago
Hello @PaulieScanlon π
can you assign me to the Wikipedia #127, Youtube #129, Gist #103, and Spotify #115 (unit tests) issues.
Hey @remiroyc I've added your name next to the components above in the Unit Test assignment list
Thanks again for getting involved!
I can take the buzzsprout and egghead ones
Hey @matiasfha I've added your name next to the components above in the Unit Test assignment list
Thanks again for getting involved!
Hey @JeffersonBledsoe you absolutely can! I'll assign you to the integration tests for now and if you feel like taking on the Unit Tests too please comment again and i'll add your name to the list.
Welcome aboard and thanks from all of us!
I forgot to say I also want to take care of the integration tests for egghead and buzzsprout component (already did xD). Also. go ahead and shoot me with anything else I can help @PaulieScanlon @spences10 happy to help you a bit more.
Hi @PaulieScanlon Can you assign me to:
Hey @xmalinov and welcome aboard! You sure can and i've added you name next to the components above. Whenever you're ready create a PR and i'll assign the Hacktoberfest label to it.
Thanks for getting involved and please reach out if you have any questions or suggestions!
Hey @xmalinov thanks for getting involved, could I ask that you maybe pick one to start with then the other issues aren't restricted for other potential contributors?
If no one else want's to pick them you can pick them up after you have completed the current one you're working on?
@spences10 noprob, lets start from the cinnamon test
@PaulieScanlon @spences10 by the way, unit tests only for presence an iframe with certain id is the bad testing scenario. The iframe can be even with wrong props (tried to check it on spotify). There is only one way to check it - like it was done in buzzsprout tests (by checking props format and types by regexps)
@matiasfha Excellent point. I'm not sure there's much we can do other testing for a string. But i do wonder if this is on the user rather than the component. Our integration tests will pick up if when a correct id is passed via a prop the provider component will load and display but the unit test is really just to ensure that the General Observer puts the child in the dom. It's a tricky situation since so much of what makes the component work is actually on the providers rather than us but we would like some kind of unit tests in there to catch any small issues.
The Buzzspout string test might be better placed in util somewhere maybe? There's a chance other components will use this method... what do you think?
@PaulieScanlon @spences10 by the way, unit tests only for presence an iframe with certain id is the bad testing scenario. The iframe can be even with wrong props (tried to check it on spotify). There is only one way to check it - like it was done in buzzsprout tests (by checking props format and types by regexps)
Ok, cool! This is good to know, so take a look at the Buzzsprout PR for guidance @xmalinov?
Also,
@spences10 noprob, lets start from the cinnamon test
I'll backtrack on what I said, @PaulieScanlon's approach will be more flexible so feel free to claim the four you suggested @xmalinov
Thanks
Hi @PaulieScanlon @spences10, may I have a go at Twitter and Twitch unit tests please?
https://github.com/PaulieScanlon/mdx-embed/issues/121 https://github.com/PaulieScanlon/mdx-embed/issues/123
Hey @ekafyi!
Thanks for joining in! Yes please do!
When you create a PR @PaulieScanlon will assign the Hacktoberfest label to it π
@PaulieScanlon I think that the regex thing in the buzzpsrout component can be generalized to some function/hook
like useValidateId(regex, id)
and returns true or false where regex
is defined by the component and id is the id used by the component.. but!.. i still the same logic to shortcircuit the rendering so is not offering anything different to the component itself. Maybe add this type of logic to the GeneralObserver
? and shortcircuit there ?
btw. I already did 4 prs. I can take more if you need or leave it for someone else that want to join to hacktoberfest. let me know
@PaulieScanlon I think that the regex thing in the buzzpsrout component can be generalized to some function/hook
like
useValidateId(regex, id)
and returns true or false whereregex
is defined by the component and id is the id used by the component.. but!.. i still the same logic to shortcircuit the rendering so is not offering anything different to the component itself. Maybe add this type of logic to theGeneralObserver
? and shortcircuit there ?
That's a great shout @matiasfha π
@PaulieScanlon thoughts??
Maybe, we can add to the testing guidelines to add a prop check for each component and the related test?
@matiasfha thanks for the feedback. I'm still not really sure if this kind of check is required. There's no way to check an id will work with any of the providers as they're just strings and i'm just kind of thinking about the usage. If a user puts in an id and nothing renders on the page they'd double check the id until it does. It'll never break their blog post if it's MDX as there's no type checking or errors. There is potential maybe to have the string check and if it errors we create a generic "error with id" alert message just so the usage isn't just an empty page but beyond that i'm not 100% sure it's needed.
With regard to the other props. This is something i have planned for future releases. The focus right now is getting at least one unit test created for each component. More granular tests will come along later. I had hoped at some point to write a url validator for each component which would check the formed url conforms to the provider specs but.... it's a big job.
To keep this moving though perhaps it's worth opening a new issue where we can discuss a more mature testing strategy because i agree it would be great to have it but attempting to implement this now will perhaps confuse the Hacktoberfest milestone goals.
Hi @PaulieScanlon - could I try out the Strava unit #117 and integration #118 tests? I'm trying to get more practice with testing in general, so I may or may not have questions. Feel free to go with another dev if you think that would be a problem!
@taylorcjohnson Ahoy and welcome! You're 100% welcome to claim those, and don't worry about skill level we're all happy to help!
To keep this moving though perhaps it's worth opening a new issue where we can discuss a more mature testing strategy because i agree it would be great to have it but attempting to implement this now will perhaps confuse the Hacktoberfest milestone goals.
That sounds good.. I would love to con tribute beyond this month with this project so.. Cound with me in that discussion.
Also if you need help or any other hacktoberfest participant just ping meπ
Hello @PaulieScanlon @spences10 I would like to work on Airtable unit and integration testing if that's ok.
Hello @PaulieScanlon @spences10 I would like to work on Airtable unit and integration testing if that's ok.
Hey @yenly, I've added your name against the issues on here, @PaulieScanlon will check it off when the PR is accepted, thanks π
Added unit test for Lbry with #162
@PaulieScanlon @spences10 I would like to work on SoundCloud unit and integration tests next. π
@yenly Hey, that would be brills. I've added your name to the components above!
@PaulieScanlon @spences10 I would be happy to help out with more tests but don't want to take them from other Hacktoberfest participants for first good issues. I'm looking to contribute beyond Hacktoberfest so let me know if these don't get picked up.
Hey @yenly Thanks, i see no harm in doing more if you'd like too. We'd welcome the help and we are just about half way through Hacktoberfest and just about half way through the issues. We may re-assign some of the above issues soon enough as some that have been claimed remain open. Just deciding on how long or what notice we should give those who asked to take on the issue but haven't created PR's yet.
Hey @PaulieScanlon @spences10 feel free to assign me some of those issues if no one else (related to hacktoberfest) claims them. Free and glad to help!
Thanks @matiasfha We're a little light on Integration Tests so if you'd like to take on a few of those that would be brills!
nice... Later tonight (my night xD) will push some prs for those. I will try to do as many as I can
@matiasfha Thanks! Can you let me know which ones and i'll add your name to the list! Ty!
Let's say CodeSandbox, Gist, Instagram, Pinterest and Spotify. I think can do at least those today
I can take Twitch & Wikipedia integration tests.
@matiasfha Lovely stuff! Thanks so much mate!
@yenly super brill, brills. Thanks so much!
Can i try the tiktok unit test? That seems to be not taken.
Hey @kriswep, you absolutely can and welcome aboard π’
I've added your name to the list above. Please do comment on the issue if you have any questions and one of us will get back to ya!
Hey @PaulieScanlon, is Vimeo's unit tests still up for grabs? #125 :)
Just added a PR for the tiktok unit test. Could also make one for Pinterest unit tests, that seems to be unassigned.
@kriswep Hey mate, i've added your name to the Pinterest unit test. Ty!
@tricinel, Hey, Vimeo unit test is all yours! Let us know if you have any questions! Thanks for getting involved!
Hey, I just send PR #188 with an unit test for whimsical.
@remiroyc Just a friendly nudge. You claimed the YouTube Unit Test, would you still like to take this on. No worries if not we can remove your name from the list and allow other contributors the opportunity to claim it.
@taylorcjohnson Just a friendly nudge. You claimed the Strava Unit Test, would you still like to take this on. No worries if not we can remove your name from the list and allow other contributors the opportunity to claim it.
@PaulieScanlon just wondering if any of the integration tests for:
are under way? They look unassigned :) Would love to get my hands dirty with cypress a bit (it's a bit new to me, so could learn something π― ).
β±οΈ Testing Umbrella Issue - Hacktoberfest
The time is right. We're now on to the last leg of the migration and something i never got round to in the old repo was adding a test suite.
π Getting Started
If you're interested in helping out with tests and taking part in Hacktoberfest please comment below and let us know if it's the unit or integration test you'd like to tackle.
We'll then assign your username/name next to the component in the list below to try and avoid duplication. Once a PR has been created we'll then be able to assign you to the actual issue which should then appear in your Hacktoberfest contributions
π€ Contributing
Before you start:
π As a thank you
We know it's not much but if you would like to contribute and your PR is successfully merged we'll send you out a super cool limited edition MDX Embed sticker. If you're comfortable sharing your address you can email me pauliescanlon@gmail.com
π§ͺ The two tests
There's two different types of tests we'd like contributors to focus on, they are at the moment fairly simple tests. There is every chance contributors with more expertise in testing will advise on what should or shouldn't be tested.
I'm open to suggestions but at the very least each component will have the below which will get us to a stable
1.0.0
releaseβ Issues
There's an issue for each component and named appropriately: e.g: Please check the issues in GitHub to see which tests are currently open or in progress
codepen-unit-test
codepen-integration-test
Each issue will be assigned the "Hacktoberfest" and perhaps the "good first issue" label?
# β Acceptance Criteria
Unit Test
Unit tests are co-located e.g:
packages/mdx-embed/src/components/codepen.test.tsx
... and here's a brief explanation of how we're writing unit testsdata-testid
that is the name of the component e.g:data-testid="codepen"
- observe directory name for clues on what thedata-testid
should bedata-testid
attribute on the top level DOM node or iframeit renders the component
.test.tsx
Integration Test
Integration tests are located at
cypress/integration/codepen.spec.js
... and here's a brief explanation about how we're currently writing integration tests.spec.js
Unit Test Assignment List π©βπ»π¨βπ»
@remiroyc@PaulieScanlonIntegration Test Assignment List π©βπ»π¨βπ»
@matiasfha@PaulieScanlonπ Phew ---- all tasks complete
GeneralObserver
- @PaulieScanlon