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: revert code coverage on home page #614

Merged
merged 4 commits into from
Aug 10, 2021
Merged

feat: revert code coverage on home page #614

merged 4 commits into from
Aug 10, 2021

Conversation

antoooks
Copy link
Contributor

@antoooks antoooks commented Aug 9, 2021

Closes #598

Description

Fixes unnecessary code coverage decrease from #604

Current Tasks

  • Remove the component rendering from the home page
  • Remove the widget from Netlify CMS
  • Update the failing tests accordingly

Kurnianto Trilaksono added 2 commits August 9, 2021 17:08
- add test for home page content
- refactor testing on component/ui/alert/**
- refactor testing on homepage-content
@netlify
Copy link

netlify bot commented Aug 9, 2021

✔️ Deploy Preview for wargabantuwarga ready!

🔨 Explore the source changes: 5f679bb

🔍 Inspect the deploy log: https://app.netlify.com/sites/wargabantuwarga/deploys/611241a6e69b2a00083bf5d5

😎 Browse the preview: https://deploy-preview-614--wargabantuwarga.netlify.app

@antoooks antoooks marked this pull request as ready for review August 9, 2021 11:15
@codecov
Copy link

codecov bot commented Aug 9, 2021

Codecov Report

Merging #614 (5f679bb) into main (4baba13) will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #614      +/-   ##
==========================================
+ Coverage   80.09%   80.17%   +0.07%     
==========================================
  Files         115      115              
  Lines        1286     1286              
  Branches      418      418              
==========================================
+ Hits         1030     1031       +1     
+ Misses        250      249       -1     
  Partials        6        6              
Impacted Files Coverage Δ
pages/index.tsx 100.00% <0.00%> (+11.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4baba13...5f679bb. Read the comment docs.

@resir014 resir014 added the automerge To be merged automatically once all the requirements are fulfilled label Aug 10, 2021
@kodiakhq kodiakhq bot merged commit 65ffdf1 into kawalcovid19:main Aug 10, 2021
Copy link
Member

@zainfathoni zainfathoni left a comment

Choose a reason for hiding this comment

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

I'm reopening the #598 issue since many testing assertion issues still need to be addressed.

<HomePage latestNews={[latestNewsItemBuilder()]} />,
);

expect(container.firstChild?.hasChildNodes()).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

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

@antoooks We should use more robust assertions instead of just asserting the existence of a child node to avoid false positive test results.

FYI Mas @rubiagatra.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok @zainfathoni, will lookout for it on the weekend

@@ -7,6 +7,6 @@ describe("homepage content render correctly", () => {
const { container } = render(<HomePageContent />);

it("renders correctly", () => {
expect(container.firstChild).toHaveClass("flex-1");
expect(container.hasChildNodes()).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

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

@antoooks We should use more robust assertions instead of just asserting the existence of a child node to avoid false positive test results.

FYI Mas @rubiagatra.

expect(screen.getByRole("button")).toHaveClass(
`close-button text-red-400 hover:text-red-500`,
);

fireEvent.click(screen.getByRole("button"));
Copy link
Member

Choose a reason for hiding this comment

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

@antoooks We should assert what should happen after the button is clicked.

FYI Mas @rubiagatra.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge To be merged automatically once all the requirements are fulfilled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove the "last updated" alert on the home page
4 participants