-
-
Notifications
You must be signed in to change notification settings - Fork 734
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
feat: add unit tests for AuthorAvatars , TOC , Hero , Meeting and DemoAnimation and Figure components #1831
feat: add unit tests for AuthorAvatars , TOC , Hero , Meeting and DemoAnimation and Figure components #1831
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-1831--asyncapi-website.netlify.app/ |
it('renders the TOC correctly with empty content', () => { | ||
mount(<TOC toc={[]} />); | ||
|
||
cy.get('.hidden').should('not.exist'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use data-tested @reachaadrika
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reachaadrika Kindly address this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akshatnema test-id is not working , here tried to use both the divs , since I am only checking for hidden which is an optional css
cypress/test/TOC.cy.js
Outdated
it('expands and collapses the TOC on click', () => { | ||
mount(<TOC toc={toc} />); | ||
|
||
// Verify initial state | ||
cy.get('.hidden').should('exist'); // TOC content should be hidden | ||
|
||
// Click on the TOC header | ||
cy.get('h5').click(); | ||
|
||
// Verify expanded state | ||
cy.get('.hidden').should('not.exist'); // TOC content should be visible | ||
|
||
// Click again on the TOC header | ||
cy.get('h5').click(); | ||
|
||
// Verify collapsed state | ||
cy.get('.hidden').should('exist'); // TOC content should be hidden |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use data-tested @reachaadrika
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this component , I am checking if hidden is present or not and there is a check for this in multiple div blocks , thus data-testid doesn't work here
cypress/test/AuthorAvatars.cy.js
Outdated
cy.get('a') | ||
.should('not.exist'); | ||
|
||
cy.get('img') | ||
.eq(index) | ||
.should('have.attr', 'src', author.photo) | ||
.should('have.attr', 'title', author.name) | ||
.should('have.class', index > 0 ? `absolute left-${index * 7} top-0` : `relative mr-${(authorsWithoutLinks.length - 1) * 7}`) | ||
.should('have.class', `z-${(authorsWithoutLinks.length - 1 - index) * 10}`) | ||
.should('have.class', 'h-10 w-10 border-2 border-white rounded-full object-cover hover:z-50'); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use data-tested @reachaadrika
cypress/test/AuthorAvatars.cy.js
Outdated
cy.get(`a[alt="${author.name}"][href="${author.link}"]`) | ||
.should('have.length', 1) | ||
.within(() => { | ||
cy.get('img') | ||
.should('have.attr', 'src', author.photo) | ||
.should('have.attr', 'title', author.name) | ||
.should('have.class', index > 0 ? `absolute left-${index * 7} top-0` : `relative mr-${(authors.length - 1) * 7}`) | ||
.should('have.class', `z-${(authors.length - 1 - index) * 10}`) | ||
.should('have.class', 'h-10 w-10 border-2 border-white rounded-full object-cover hover:z-50'); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not data-tested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please go through the comments, and run through the code.
You should be using data-testid
..
@reachaadrika Kindly update the PR title related to changes inside PR |
components/Feedback.js
Outdated
@@ -71,7 +71,7 @@ export default function Feedback(className = '') { | |||
<div className='block mx-auto w-fit'> | |||
<img src='/img/illustrations/icons/icon-x.svg' className='md:w-14' /> | |||
</div> | |||
<div className='text-center mx-auto text-lg mt-4'> | |||
<div className='text-center mx-auto text-lg mt-4' data-testid="Feedback-div2"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making naming convention with 1,2,3.. as suffux is not good practice, kindly update it with good naming for data-testid
.
@@ -50,7 +50,7 @@ export default function Feedback(className = '') { | |||
<div className='block mx-auto w-fit'> | |||
<img src='/img/illustrations/icons/icon-check.svg' className='md:w-14' /> | |||
</div> | |||
<div className='text-center mx-auto text-lg mt-4'> | |||
<div className='text-center mx-auto text-lg mt-4' data-testid="Feedback-div"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unit test for this component is not added in PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this component , I require another file namely Routers which is in another PR once that is merged I will push the tests for this component , since pushing this separately can cause conflicts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So,on which PR, these particular changes are dependent on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Button className="block md:inline-block" text="Read the docs" href="/docs" icon={<ArrowRight className="-mb-1 h-5 w-5" />} | ||
data-testid="Hero-Button"/> | ||
{/* Wrap SearchButton with AlgoliaSearch component */} | ||
<AlgoliaSearch> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify the reason for wrapping the Algolia Search in this component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here will writing the tests for Hero , Search Button was not able to access the context , and some other refs , thus wrapped AlgoliaSearch component around it
/rtm |
Hello, @akshatnema! 👋🏼 |
/rtm |
Description
1. AuthorAvatars : This is a standalone file in the code , which tells and shares the Author details . For this component I have included following tests :
2. TOC : This is a standalone file in the code , I have included following testcases for this :
-renders the TOC correctly
3. Hero : This is also a standalone file , and has the main hero component in the code . For this component I have included following tests :
4. Meeting : This is also a standalone file . I have included following test cases .
5.Figure : This is also a standalone file .
Related issue(s)
fixes #1830