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

feat: compute name takes in network #62

Merged
merged 2 commits into from
Dec 20, 2024
Merged

Conversation

joseph-sentry
Copy link
Contributor

@joseph-sentry joseph-sentry commented Dec 19, 2024

  • this covers the case where a user will upload their test results file
    for pytest without -o junit_family=legacy, but with their network set
  • this is the network that is usually uploaded from the CLI
  • the network is optional argument and defaults to None
  • since it's usually a list of file paths that contain the paths to
    source files we can use it to deduce the path associated with a pytest
    test result if the filename has not been provided
  • this commit also moves the tests from python to rust and removes the
    python interface for the compute_name function since it's no longer
    needed
  • there's possibly future work that can be done to optimize this process
    of matching the test result classname to files in the network, but
    this is a minimal working solution, it's also possible that with the
    updated instructions in the docs

Depends on: #61

@joseph-sentry joseph-sentry marked this pull request as draft December 19, 2024 22:04
@codecov-notifications
Copy link

codecov-notifications bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.68%. Comparing base (a1636b3) to head (d430923).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
+ Coverage   86.79%   88.68%   +1.88%     
==========================================
  Files           5        5              
  Lines         606      707     +101     
==========================================
+ Hits          526      627     +101     
  Misses         80       80              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joseph-sentry joseph-sentry marked this pull request as ready for review December 19, 2024 22:20
@joseph-sentry joseph-sentry requested a review from a team December 19, 2024 22:20
@Swatinem
Copy link
Contributor

lgtm, though would have been nice to split the changes to compute_name and the added method to understand the error/failure message into different PRs.

- this covers the case where a user will upload their test results file
  for pytest without -o junit_family=legacy, but with their network set
- this is the network that is usually uploaded from the CLI
- the network is optional argument and defaults to None
- since it's usually a list of file paths that contain the paths to
  source files we can use it to deduce the path associated with a pytest
  test result if the filename has not been provided
- this commit also moves the tests from python to rust and removes the
  python interface for the compute_name function since it's no longer
  needed
- there's possibly future work that can be done to optimize this process
  of matching the test result classname to files in the network, but
  this is a minimal working solution, it's also possible that with the
  updated instructions in the docs
@joseph-sentry joseph-sentry merged commit 996ecb2 into main Dec 20, 2024
10 of 11 checks passed
@joseph-sentry joseph-sentry deleted the joseph/compute-name branch December 20, 2024 16:26
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.

2 participants