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

Use test paths relative to package directory. #12751

Merged
merged 6 commits into from
Mar 4, 2023

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Feb 9, 2023

Description

This PR standardizes how our test paths are defined for dask-cudf and custreamz. This was originally a test to investigate a question related to CodeCov, which is answered below. We noticed that some coverage reports in the CI outputs used full paths like /opt/conda/envs/test/lib/python3.10/site-packages/cudf/__init__.py while other coverage reports gave relative paths like cudf/__init__.py. The net result of this PR is that coverage reports are unchanged, but our CI tests are more correct because they always pull from the installed packages rather than the local packages (import dask_cudf is fetching the built conda packages from site-packages and not importing from the current path in the cloned repo).

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@github-actions github-actions bot added the ci label Feb 9, 2023
@ajschmidt8 ajschmidt8 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 9, 2023
@bdice
Copy link
Contributor Author

bdice commented Feb 10, 2023

@ajschmidt8 I think the absolute paths must be interpreted correctly by CodeCov. In commit 002fba0, I changed the test paths but forgot to modify the path to .coveragerc. Then in commit 689e67e, I fixed that problem and the total coverage is unchanged relative to the base branch. I would like to merge this PR since it makes our path handling more standard across packages and adds a missing .coveragerc file for custreamz.

@bdice bdice marked this pull request as ready for review February 10, 2023 21:24
@bdice bdice requested review from a team as code owners February 10, 2023 21:24
@galipremsagar
Copy link
Contributor

@bdice Should we wait for codecov bot to comment the reports on this PR before merging?

@bdice
Copy link
Contributor Author

bdice commented Feb 10, 2023

@bdice Should we wait for codecov bot to comment the reports on this PR before merging?

I don't know why the bot hasn't commented yet. I will merge in branch-23.04 and see if a new commit triggers the bot since I opened this PR from draft.

@bdice
Copy link
Contributor Author

bdice commented Feb 10, 2023

Hard to know what's wrong. @galipremsagar Can you tell? https://app.codecov.io/gh/rapidsai/cudf/pull/12751

@galipremsagar
Copy link
Contributor

Hard to know what's wrong. @galipremsagar Can you tell? https://app.codecov.io/gh/rapidsai/cudf/pull/12751

The error shown in that page that base branch is missing is related to our nightly jobs not running probably.

But the bot should have commented even if above is the case, lets wait and see if the bot comments in a bit.

@ajschmidt8
Copy link
Member

The error shown in that page that base branch is missing is related to our nightly jobs not running probably.

For reference, we generally don't enable nightly builds/tests for the next development branch until the release is complete.

So 23.04 nightly builds/tests will begin next week.

@ajschmidt8
Copy link
Member

I still don't get why in some coverage results (like this), it reports relative paths like:

kafka.py 

but in that same log (here), it shows other paths absolutely like:

/opt/conda/envs/test/lib/python3.10/site-packages/cudf/core/buffer/utils.py

@bdice
Copy link
Contributor Author

bdice commented Mar 4, 2023

/merge

@rapids-bot rapids-bot bot merged commit 45a9e82 into rapidsai:branch-23.04 Mar 4, 2023
@vyasr
Copy link
Contributor

vyasr commented Mar 6, 2023

@bdice were the above issues with the codecov bot resolved?

@bdice
Copy link
Contributor Author

bdice commented Mar 6, 2023

@vyasr I don't see Codecov bot PR comments on these recent PRs, so it's not related to the changes in this PR (which seemed fine, I verified them).

I don't know how to address this, it probably takes repo admin privileges if it's an issue with the application setup or webhooks. cc: @rapidsai/ops

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants