Skip to content

Feat/mini challenge 4#84

Open
lesliedayann wants to merge 12 commits into
cvillanueva84:masterfrom
lesliedayann:feat/mini-challenge-4
Open

Feat/mini challenge 4#84
lesliedayann wants to merge 12 commits into
cvillanueva84:masterfrom
lesliedayann:feat/mini-challenge-4

Conversation

@lesliedayann

Copy link
Copy Markdown

No description provided.

it('should to take snapshop', () => {
expect(wrapper).toMatchSnapshot();
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is on this test the App component fetching the real YouTube Api? seems like that. Remember to avoid real YouTube api calls on tests.

const history = useHistory();
const { search, setSearch, darkMode, setDarkMode } = useContext(AppContext);
const [anchorEl, setAnchorEl] = useState(null);
// const [darkMode, setDarkMode] = useState(false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

always remove commented code

const darkMode = screen.getByText(/Dark Mode/i);
expect(darkMode).toBeInTheDocument();
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good test :)

const thumbnail = screen.getByRole('img');
expect(thumbnail).toHaveAttribute('src', props.imageURL);
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good test :D

test('Render RelatedVideo component', () => {
expect(wrapper).not.toBeNull();
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could add more expects like a related video values

const descriptionElement = screen.getByText(correctProps.description);
expect(descriptionElement).toBeInTheDocument();
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good test

test('Render Home Page', () => {
expect(wrapper).not.toBeNull();
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is on this test fetching the real YouTube api?

test('Render Video Template Page', () => {
expect(wrapper).not.toBeNull();
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is on this test fetching the real YouTube api?

@ghost

ghost commented Aug 21, 2021

Copy link
Copy Markdown

Acceptance Criteria

  • The search term is stored and retrieved from the Global Context correctly.
  • The appearance theme is stored on the Global Context and applied correctly to the App UI.
  • useReducer hook is implemented correctly to manage the Global State.

Bonus Points

  • Testing coverage is above 70%. (Please include a screenshot of the code coverage report in your PR description).

Seems like is not present the useReducer, if there is, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants