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

Solução parcialmente adicionada #1581

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

CamilleHaus
Copy link

DEMO LINK

Eu sei que não está passando todos, mas não estou entendendo o último erro e como fazer os testes pausados rodarem

Copy link

@pedro-ruas pedro-ruas left a comment

Choose a reason for hiding this comment

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

I pointed to the possible cause of errors, there is a mismatching User_ID passed to the API, check which is the correct one for you

src/App.tsx Outdated
.finally(() => {
dispatch({ type: 'stopUpdate' });
});
}, []);

Choose a reason for hiding this comment

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

Add dispatch as a dependency

import { Todo } from '../types/Todo';
import { client } from '../utils/fetchClient';

export const USER_ID = 2165;

Choose a reason for hiding this comment

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

Check this user here, you are setting the UserID (should be unique to you, Camille, not for who is using the website) to 2165.
Later on, you are passing a different number

type: 'addTempTodo',
payload: { userId: 962, id: 0, title: title.trim(), completed: false },
});
addTodo({ userId: 962, title: title.trim(), completed: false })

Choose a reason for hiding this comment

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

Here you are sending to the API an Todo that belongs to the UserId 962, which is a different number.
If the numbers mismatch, you are probably trying to load your todos in the GET method, but saving them to another user, so they won't be found by you.

@CamilleHaus
Copy link
Author

Fiz as alterações, @pedro-ruas mas ele ta reprovando em um teste dizendo "Should stay open on fail"

@CamilleHaus CamilleHaus requested a review from pedro-ruas January 4, 2025 21:49
Copy link

@joaorpereira joaorpereira left a comment

Choose a reason for hiding this comment

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

Before making the changes, run dpeloy again and verify if the error persists on the pipeline.

If the error persists, let's try the change that I shared to see if changing that fixes the issue. Do not forget to run deploy command.

If the issue persists please send to me a message inside mate platform for me try to find a solution.

I run the test locally and all were passing

/>
</label>

{isEditing && selectedTodo === todo.id ? (

Choose a reason for hiding this comment

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

try to remove the logic selectedTodo === todo.id and deploy again the PR. Let's see if doing that the test pass

@@ -519,7 +519,7 @@ describe('', () => {
});

// this test may be flaky
it.skip('should replace loader with a created todo', () => {
it('should replace loader with a created todo', () => {

Choose a reason for hiding this comment

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

why you change this test?

@@ -1515,7 +1515,7 @@ describe('', () => {
});

// It depend on your implementation
it.skip('should stay while waiting', () => {
it('should stay while waiting', () => {

Choose a reason for hiding this comment

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

why you change this test?

@@ -1694,7 +1694,7 @@ describe('', () => {
});

// this test may be unstable
it.skip('should hide loader on fail', () => {
it('should hide loader on fail', () => {

Choose a reason for hiding this comment

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

why you change this test?

@CamilleHaus
Copy link
Author

@joaorpereira De fato joão, apenas rodar o deploy fez com que tudo passasse. Muito Obrigada!

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.

3 participants