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

feat: add unit tests for campaigns folder #1724

Merged
merged 48 commits into from
Jul 17, 2023

Conversation

reachaadrika
Copy link
Contributor

@reachaadrika reachaadrika commented May 29, 2023

Description

  • Here the unit tests have been added for the 'campaigns' component of the AsyncAPI , which displays recent campaign banners and announcements , for eg : the Madrid conference 2023 . This component has 3 files namely :

  • Banner.js : This showcases the event banner or the announcement banner . Therefore there were 3 major unit tests that were written for this page :

    1. It should render the banner when the date is within the valid range .
    2. should display the correct message when the date is within the valid range
    3. should have a link to the recordings playlist
      For this in the current system , 'All the tests have been passed '
      Below is the result :

image

These tests pass since right now we do not have the need to display this ended banner , instead we have to display the Announcement banner for Madrid .

  • AnnouncementRemainingDays.js : Countdown . The tests included here are :

    1. displays correct countdown text
      For this test , one can add the specific dateTime and eventName for the event . Currently I have taken arbitary values for the same .
      image
      These tests pass and have displayed current no. of days .
  • AnnouncementHero.js : Main poster for the conference . The tests included here are :

    1. should render the banner when it should be shown
    2. should not render the banner when it should be hidden
      For now test 2 fails since there is no class present and the banner has to be shown to the users .
      image

These changes do not introduce any breaking changes .

Related issue(s)
Fixes #1718

@netlify
Copy link

netlify bot commented May 29, 2023

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 6027dd5
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/64b415ac05917f0008ff4304
😎 Deploy Preview https://deploy-preview-1724--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions
Copy link

github-actions bot commented May 29, 2023

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 39
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🔴 PWA 30

Lighthouse ran on https://deploy-preview-1724--asyncapi-website.netlify.app/

@reachaadrika reachaadrika changed the title unit tests for campaigns folder adding unit tests for campaigns May 29, 2023
@reachaadrika reachaadrika changed the title adding unit tests for campaigns adding new unit tests for campaigns May 29, 2023
@reachaadrika reachaadrika changed the title adding new unit tests for campaigns Adding unit tests for campaigns folder May 29, 2023
@reachaadrika reachaadrika changed the title Adding unit tests for campaigns folder feat: adding unit tests for campaigns folder May 29, 2023
@akshatnema akshatnema added the gsoc This label should be used for issues or discussions related to ideas for Google Summer of Code label Jun 1, 2023
Copy link
Member

Choose a reason for hiding this comment

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

This file is already deleted in another PR. It may create issues while merging any of the PRs.

} else {
mount(<Banner />);
// cy.wait(1000); // Adjust the wait time as needed
cy.get('.bg-gray-100').should('be.visible');
Copy link
Member

Choose a reason for hiding this comment

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

also check for the .max-w-screen for this component, because it's an important UI aspect. It should cover the whole screen in terms of width.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done the required changes in current commit .

}
}

cy.get('.countdown-text-gradient').should('have.text', `${text} until ${eventName}`);
Copy link
Member

Choose a reason for hiding this comment

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

Also check for font-extrabold class because it's an announcement. No one in the future should remove the class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done the required changes in current commit .

@akshatnema akshatnema requested a review from imabp June 3, 2023 07:15

if (year === 2022 && month === 10 && day >= 6) {
mount(<Banner />);
//cy.wait(1000); // Adjust the wait time as needed
Copy link
Member

Choose a reason for hiding this comment

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

Why the wait time is included in the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Included the wait time earlier , had to test something specific , removed it , though I will remove the comments .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@akshatnema made a new commit removing the wait time , I am sorry for the commit message here , VS code did it , I was not able to connect to git , so had to push through source control in vs .

mount(<AnnouncementHero />);

// Assert that the component is rendered
cy.get('.bg-gray-50').should('exist');
Copy link
Member

@imabp imabp Jun 4, 2023

Choose a reason for hiding this comment

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

We can use, data-componentName, in the original component, and use it in cypress for more accurate testing?
cc: @akshatnema @reachaadrika

For example:

<button
  id="main"
  class="btn btn-large"
  name="submission"
  role="button"
  data-cy="submit"
>
  Submit
</button>

Cypress Code

cy.get('[data-cy="submit"]')

Benefits of using data-ComponentId, it is isolated from all the JS CSS changes, (post css as well) offers more accuracy to our testing.

Refer this Cypres documentation

Copy link
Member

Choose a reason for hiding this comment

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

I think @reachaadrika is using it at many places. Maybe she forgot about it here, but yeah, the test for background color should be there as it tests whether component is rendering it's properties also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am checking for background and for rest of the components I will add data attribute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done the suggested changes , do check @imabp

Copy link
Member

@imabp imabp Jun 8, 2023

Choose a reason for hiding this comment

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

cy.get('.bg-gray-50').should('exist');

@reachaadrika still the change is not reflected here
Kindly update

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@imabp imabp left a comment

Choose a reason for hiding this comment

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

Let's use data-componentName, which is isolated from all the changes, that might happen as part of CSS and JS manipulations.

Please refer this section of doc, on how to use it

mount(<AnnouncementHero />);

// Assert that the component is not rendered
cy.get('.bg-gray-50').should('not.exist');
Copy link
Member

Choose a reason for hiding this comment

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

same here as well, using data-componentName
ref

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure @imabp adding data-cy and I will keep in mind to add data-cy rather than relying on tags and classes

Copy link
Member

Choose a reason for hiding this comment

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

Yes @reachaadrika, please replace the implementation this with data-cy="nameOfTheComponent", and this will be helpful when we check complex properties of an element. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done the suggested changes , do check @imabp

@@ -27,16 +27,16 @@ export default function AnnouncementHero({ className = '', small = false, hideVi
<Container wide as="section" padding='' className='text-center'>
<div
className={`bg-gray-50 border border-gray-200 py-6 rounded ${className} ${
small ? 'mb-4' : 'mx-3 mt-3 p-3 mb-6'
}`}
small ? 'mb-4' : 'mb-12'
Copy link
Member

Choose a reason for hiding this comment

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

@reachaadrika Why did you changed the CSS here?
You should check for these classNames inside the test file instead of reverting it to previous classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

const today = new Date();
const [day, month, year] = [today.getUTCDate(), today.getUTCMonth(), today.getUTCFullYear()];
if (year > 2022 || month !== 10 || day < 6) {
mount(<Banner />);
Copy link
Member

Choose a reason for hiding this comment

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

Use beforeEach here to mount the Banner Component

Copy link
Contributor Author

@reachaadrika reachaadrika Jul 16, 2023

Choose a reason for hiding this comment

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

For this I can't use BeforeEach since , for some parts in the code , I am rendering Banner after the clock is set , to get the Banner displayed like here const mockDate = new Date(2021, 10, 12).getTime(); cy.clock(mockDate); mount(<Banner />);

@akshatnema akshatnema changed the title feat: adding unit tests for campaigns folder feat: add unit tests for campaigns folder Jul 16, 2023
@akshatnema
Copy link
Member

/rtm

@asyncapi-bot asyncapi-bot merged commit 3708aef into asyncapi:master Jul 17, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc This label should be used for issues or discussions related to ideas for Google Summer of Code ready-to-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unit Tests for 'campaigns' folder and the components within it .
4 participants