search

nus-cs2103-AY2122S2 / forum

13 stars 1 forks source link

CI workflow error #203

Closed hsiaojietng closed 2 years ago

hsiaojietng commented 2 years ago

I am trying to fix some errors during integration.

These are some of the errors I face: image

It shows that I have a test case that is failing, however, when I run test on gradle on my side, everything passes, really do not understand why I am failing that test case on EditCommand.

Similarly, for the JacocoTestReport, when I run it on my side, no errors at all, but this ambiguous error pops up when it is run as a CI workflow and I am not even sure how to solve this error as nothing is provided.

Please help! You can take a look at my code here

yusufaine commented 2 years ago

@hsiaojietng can I just check, do you get an error when you run ./gradlew test (or what the Window's version is)? I'm asking because jacocoTestReport just compiles the errors that went wrong, so I don't think that's the issue per se.

hsiaojietng commented 2 years ago

@yusufaine I ran everything under 'Verification' of gradle and all tests run successfully as shown image

Test (with warnings but no errors): image

Jacoco Test Report image

Jacoco Test Coverage Verification image

and Check (with warnings but not errors): image

hsiaojietng commented 2 years ago

By the way, as you guys can see from the first screenshot, there are 250 tests passed and 1 failed. Locally, when I run test checks, it only shows 250 test passed and that the test runs successfully. As shown below: image Really boggles my mind what is wrong

pyk595 commented 2 years ago

Hi, I think it's an error that arises when github tries to test your code after simulating the merge with master. The test you're having problem with is this: seedu.address.logic.commands.EditCommandTest > execute_allFieldsSpecifiedUnfilteredList_success() FAILED Check if there are any changes to this test from your team repo

pyk595 commented 2 years ago

@hsiaojietng I took a rough glance at your repo, you probably need to resolve the differences between your editedPerson and your editPersonDescriptor

hsiaojietng commented 2 years ago

Hi @pyk595, thanks for the tip, really helped!

hsiaojietng commented 2 years ago

Please help, I really don't know what to do anymore.

I am facing the same issue, failing the test case seedu.address.logic.commands.EditCommandTest > execute_allFieldsSpecifiedUnfilteredList_success() FAILED again but this time I thought I got it.

image Basically, at first, the value for favourite for descriptor object was Optional.empty() and value for favourite for editPersonDescriptor object was null, therefore I thought changing that would solve the test.

But when I tried to re-run the workflow after pushing from my branch, it still shows the same test error as shown in this PR: https://github.com/AY2122S2-CS2103-W16-4/tp/pull/80.

This is the current version of the branch I am working on: https://github.com/hsiaojietng/tP/tree/add-favourite-feature

Please help this is really annoying that I don't even know what is wrong.

damithc commented 2 years ago

@hsiaojietng is it a case of 'passing locally but failing in CI'? In that case, ensure your local branch is synced with the latest master of the team repo, because GitHub merges the master branch to your branch before running the tests.

hsiaojietng commented 2 years ago

@damithc I just did but I'm still facing the same issue

damithc commented 2 years ago

Was this intended? image

To sync a local branch with the upstream master,

  1. pull the latest master branch from the team repo
  2. in the local repo, merge the master branch to your branch
  3. run tests locally (debug and failures, if any)
  4. push the branch to your fork
hsiaojietng commented 2 years ago

Yup that was unintended, it has been reversed. I did the steps exactly but still failing that 1 test case execute_allFieldsSpecifiedUnfilteredList_success()

damithc commented 2 years ago

Yup that was unintended, it has been reversed. I did the steps exactly but still failing that 1 test case execute_allFieldsSpecifiedUnfilteredList_success()

I see. You can push that code to your branch so that someone can try to look at the code and see what's wrong. In the meantime, you can use the following steps to try to locate the problem yourself.

  1. locate the failing test in Intellij, and run it by itself. confirm it is still failing.
  2. find out the expected and actual behavior (as reported by the test failure). Confirm that the problem is not in the test itself (perhaps the test should be updated to match the code update you did?)
  3. add a breakpoint in a suitable location in the code and use the Intellij debugger to step-through the code until you figure out where the code produces the wrong behavior.
hsiaojietng commented 2 years ago

Thanks @damithc, I have done those checks but still unable to find the problem. I was thinking of 1) Finding the first commit that might have failed and creating a detached head from that commit. 2) Try to locate the problem and solve it. 3) PR into the master branch to check if the CI workflow passes. 4) If so, I delete the PR, make the necessary changes to my latest favourite-feature branch and then PR into the master branch of the team repo. 5) Else, I continue checking by repeating the steps for the next commit until I reach the commit where I failed the CI workflow What do you think? PS. I am just afraid that doing so might cause some issues since I am unsure of how detached heads work.

damithc commented 2 years ago

Thanks @damithc, I have done those checks but still unable to find the problem. I was thinking of

  1. Finding the first commit that might have failed and creating a detached head from that commit.
  2. Try to locate the problem and solve it.
  3. PR into the master branch to check if the CI workflow passes.
  4. If so, I delete the PR, make the necessary changes to my latest favourite-feature branch and then PR into the master branch of the team repo.
  5. Else, I continue checking by repeating the steps for the next commit until I reach the commit where I failed the CI workflow What do you think? PS. I am just afraid that doing so might cause some issues since I am unsure of how detached heads work.

@hsiaojietng You can do the first 2 steps. Once you locate the problem, most likely you can go back to the normal branch HEAD and fix the problem there as the same problem is likely to be causing the problem even in the current HEAD commit.

hsiaojietng commented 2 years ago

Hi @damithc, I tried doing both step 1 and 2. I fixed the problems and PR into master branch of team repo but faced some minor checkstyle errors (guessing it's from when I did merging of conflicts since I did not encounter any errors locally). Is there any way to make edits to merge conflicts that I previously made changes to?

damithc commented 2 years ago

Hi @damithc, I tried doing both step 1 and 2. I fixed the problems and PR into master branch of team repo but faced some minor checkstyle errors (guessing it's from when I did merging of conflicts since I did not encounter any errors locally). Is there any way to make edits to merge conflicts that I previously made changes to?

@hsiaojietng you can run checkstyle locally with the gradlew verify command (which includes checkstyle check as well). Once you find the error, you can fix it as a new commit (no need to edit the past commit).

hsiaojietng commented 2 years ago

Thanks, still having same test case failure in CI. Would you allow me to restart everything? (Create a new repo, forked from my current team repo, thus all existing commits of mine are gone?)

damithc commented 2 years ago

Thanks, still having same test case failure in CI. Would you allow me to restart everything? (Create a new repo, forked from my current team repo, thus all existing commits of mine are gone?)

There is no restriction against that. But note that we look at commit timestamps to determine your activity level in each week.

hsiaojietng commented 2 years ago

Thanks!