Add unit tests for logfire and http output methods.#988
Conversation
e8dc95e to
d0ff5e4
Compare
|
Not sure what happened here, maybe I messed pu my branches. |
|
One thing: I see this failure https://github.com/mlco2/codecarbon/actions/runs/19781662666/job/56682713399 complaining: This works in my local environment I did add logfire to to dev dependencies in pyproject.toml, but maybe that wasn't enough? |
|
Interesting issue!
TLDR: the test should fail in master as expected because there is no logfire unless the user installs it😄 Thanks @cianc for spotting this! Solutions:
There is a similar issue with |
|
Another option: can we add logfire to this line where we're already installing |
I totally agree : we need to have minimal dependencies for default install (reduce dependency hell and lower attack surface) and provide optional dependencies for output method. |
Description
Add unit tests for logfire and http output methods.
This requires adding logfire as a dev dependency so we can import it in tests.
Also make it clearer which
EmissionsDataparams are unnecessary for theoutandlive_outmethods.Related Issue
Please link to the issue this PR resolves: #972
Motivation and Context
Increases test coverage.
How Has This Been Tested?
Ran all existing and new tests.
Screenshots (if appropriate):
Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply:Checklist:
Go over all the following points, and put an
xin all the boxes that apply.