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

How to debug / configure different comment races to overwrite each? #622

Open
kwonoj opened this issue Jan 10, 2025 · 3 comments
Open

How to debug / configure different comment races to overwrite each? #622

kwonoj opened this issue Jan 10, 2025 · 3 comments

Comments

@kwonoj
Copy link

kwonoj commented Jan 10, 2025

Describe the bug

Current setup includes uploading test coverage, test results, and the bundle analysis. Depends on the ci action running order, I am observing some information does not stay: In some cases, comment only contains bundle analysis. For the other cases, coverage or test result appears. Couldn't find the exact condition when / how / what appears unfortuntely.

Per recommendtation codecov.yaml have a configuration for manual trigger (https://docs.codecov.com/docs/codecovyml-reference#codecovnotifymanual_trigger), but that doesn't seem to working - even before codecov cli uploader runs send-notifications, can see the comment appears. Peeking logs (using self hosted 25.1.3) it seems notification is triggered while config sees manual_trigger correctly.

{
  "message": "We are going to be sending notifications",
  "asctime": "2025-01-10 17:15:32,246",
  "name": "tasks.notify",
  "levelname": "INFO",
  "lineno": 393,
  "pathname": "/worker/tasks/notify.py",
  "funcName": "run_impl_within_lock",
  "threadName": "MainThread",
  "taskName": null,
  "commit": "b7c5fd987940cf66b53f61e0ea0b238a0b382205",
  "repoid": 214,
  "current_yaml": {
    "codecov": {
      "require_ci_to_pass": true,
      "notify": {
        "wait_for_ci": true,
        "manual_trigger": true
      },
      "ci": [
        "!appveyor"
      ]
    },
    "coverage": {
      "precision": 2,
      "round": "down",
      "range": [
        60.0,
        80.0
      ],
      "status": {
        "project": {
          "default": {
            "informational": true,
            "only_pulls": true
          }
        },
        "patch": {
          "default": null,
          "informational": true
        },
        "changes": false,
        "default_rules": {
          "flag_coverage_not_uploaded_behavior": "include"
        }
      }
    },
    "comment": {
      "layout": "condensed_header, components, condensed_files, condensed_footer",
      "hide_project_coverage": false,
      "behavior": "default",
      "show_carryforward_flags": false,
      "require_changes": [
        6
      ]
    },
    "slack_app": true,
    "github_checks": {
      "annotations": true
    },
    "to_string": "codecov:\n  ci:\n    - \"!appveyor\"",
    "component_management": {
      "default_rules": {
        "statuses": [
          {
            "type": "project",
            "target": "auto",
            "branches": [
              "^!main$"
            ]
          }
        ]
      },
      "individual_components": [
        {
          "component_id": "app",
          "name": "app",
          "paths": [
            "(?s:src/.*)\\Z"
          ]
        },
        {
          "component_id": "math",
          "name": "@math",
          "paths": [
            "(?s:packages/math/.*)\\Z"
          ]
        }
      ]
    },
    "bundle_analysis": {
      "status": "informational"
    }
  },
  "context": {
    "task_name": "app.tasks.notify.Notify",
    "task_id": "694a0f10-e2fa-430c-9e15-33d552134ddf",
    "owner_username": "[redacted]",
    "owner_service": "github",
    "owner_plan": "users-basic",
    "owner_id": 1,
    "repo_name": "[redacted]-Web",
    "repo_id": 214,
    "commit_sha": "b7c5fd987940cf66b53f61e0ea0b238a0b382205",
    "commit_id": null,
    "sentry_trace_id": null
  },
  "logger.name": "tasks.notify",
  "logger.thread_name": "MainThread",
  "level": "INFO"
}

Environment (please complete the following information):
github actions

To Reproduce

  • create a monorepo setup that runs codecov cli have multiple upload process, like
//run_test.yml
codecov --auto-load-params-from GithubActions -u https://selfhosted.com upload-process --handle-no-reports-found --report-type test_results  --disable-search -f test-results/junit.xml --flag jest; fi

codecov --auto-load-params-from GithubActions -u https://selfhosted.com upload-process --handle-no-reports-found --disable-search -f ./coverage/cobertura-output.xml

//run_bundle.yml
webpack

//webpack.config
codecovWebpackPlugin({
      apiUrl: 'https://selfhosted.com',
      enableBundleAnalysis: process.env.CODECOV_TOKEN !== undefined,
      bundleName: 'app',
      gitService: 'github',
      telemetry: false,
      uploadToken: process.env.CODECOV_TOKEN,
    }),

//notify.yml

needs: [run_test, run_bundle]
run: |
  codecov --auto-load-params-from GithubActions -u https://selfhosted.com send-notifications -r org/repo

Expected behavior
Expect notification appears when trigger send-notifications, which shows aggregated results for test coverage / analytics / bundle

Screenshots
Only one info appears, depends on the trigger timing

Image

Additional context

Self hosted, 25.1.3.

@covecod covecod bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 Jan 10, 2025
@giovanni-guidini
Copy link

Hello @kwonoj,

Thank you for your patience here... are you still experiencing this problem?

I think there might be some wrong assumptions as to the "expected behavior". The bundle analysis (BA) comment is completely separate from the notification one. If I'm not mistaken the send-notification should only affect the coverage + test analytics (TA) comment, with the BA comment being executed automatically (there's no "send-notifications" for it)
You can see here that there are different IDs for the comments.

I am not discarding a bug in this scenario, of course.

In your example you showed a TA comment and shared the log for the coverage + TA notify task.

Have you had the BA comment also show up?
One of the logs for it is "Starting bundle analysis notify". Can you confirm that you can see that and that the BA notify task is also succeeding?

@kwonoj
Copy link
Author

kwonoj commented Jan 24, 2025

Hi, thanks for the explanation.

I revised all of my current workflow, so hard to confirm logs or behavior again with the latest: but the q in general is regardless of bundle analysis comment, I saw the test / coverage comment appears before I manually run send-notification. Would like to confirm if that's the expected cases.

@giovanni-guidini
Copy link

giovanni-guidini commented Jan 27, 2025

Hummm... I think that the comment should not appear before running the send-notification command. But I also think that we might skip that check in the case of tests failing or some other processing issue.

And if a comment does exist already it will be updated as new information comes through the processing pipeline (maybe more than once)

UPDATE
I have confirmed with the team that the Test Analytics comment will ignore the manual_trigger if there are test failures or if there is a processing issue.

So if that is what you are experiencing then it would be expected behavior... In my personal opinion it makes sense to let users know of errors / issues as they appear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for: Product Owner
Development

No branches or pull requests

2 participants