uprm-inso4117-2023-2024-s2 / semester-project-study-pet

semester-project-study-pet created by GitHub Classroom
1 stars 3 forks source link

added aditional tests to the file #337

Closed CaribbeanCool closed 2 months ago

CaribbeanCool commented 2 months ago

I took the eat page test that was made by @diego-velez-upr, and since it had only 1 test, I added other tests and refactored the code

diego-velez-upr commented 2 months ago

The changes seem fine, but can we get @gabrielpadilla7 to test them out and approve? since I'm not at my PC anymore, if he approves then its fine by me

gabrielpadilla7 commented 2 months ago

Checking right now

gabrielpadilla7 commented 2 months ago

@CaribbeanCool Any updates on this PR?

CaribbeanCool commented 2 months ago

@gabrielpadilla7 My bad. The last two fail since the pets hunger is full. The tests work based on the web app specifically. This means that the tests don't bypass certain things, like the pet's stats

gabrielpadilla7 commented 2 months ago

@CaribbeanCool I see, so the tests will always fail or does it need to be in another environment in order for them to pass?

CaribbeanCool commented 2 months ago

@CaribbeanCool I see, so the tests will always fail or does it need to be in another environment in order for them to pass?

@gabrielpadilla7 It doesn't need to be in another environment. The test will fail if the pets hunger is above 10. For example, if you change the pet's hunger to 11 in Pet.jsx, the message "You are full!" will disappear, and the minigame would be available, meaning that the test fails when you run it.

diego-velez-upr commented 2 months ago

Is there a way that you could change the hunger before the tests run, so that way the tests pass successfully?

I don't think it'd be a good idea to submit the tests if they fail everytime.

PinkSylvie commented 2 months ago

We could change the default instead of 0 to a higher amount. I think in Pet.jsx?

CaribbeanCool commented 2 months ago

@PinkSylvie Since I haven't found a way to change the hunger in the tester itself, and Diego wants the tester to run without tests failing, do I remove the full test?

PinkSylvie commented 2 months ago

I dont think you need to change the hunger in the tester, but in the default pet the hunger could be changed

CaribbeanCool commented 2 months ago

I dont think you need to change the hunger in the tester, but in the default pet the hunger could be changed

But if you change the hunger, either one test fails or two test fail

diego-velez-upr commented 2 months ago

I dont think you need to change the hunger in the tester, but in the default pet the hunger could be changed

I think normally you want to change the test to fit the production code instead of the other way around, but if it isn't a big deal, doesn't break components or mess with the intended workflow of the app, then it might be fine.

Either way, I think that we shouldn't submit failing tests.

PinkSylvie commented 2 months ago

It might be for the best to delete them then

CaribbeanCool commented 2 months ago

It might be for the best to delete them then

The tests themselves or simply the full one?

PinkSylvie commented 2 months ago

The tests themselves, we don't want to submit failing tests

CaribbeanCool commented 2 months ago

@PinkSylvie Ok. Can i close the PR then?

PinkSylvie commented 2 months ago

Yes