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

Implements trace for failures #422

Merged
merged 20 commits into from
Jun 18, 2020

Conversation

thepabloaguilar
Copy link
Member

All context can be found on #409 issue!

Just a little explanation about _get_trace function:

def _get_trace(_self) -> List[FrameInfo]:
    current_stack = stack()
    return current_stack[2:]

The return is the current call stack starting from the third position, to avoid two non useful calls on the call stack and you can see in the example below what calls they're:

Complete call stack

/home/pabloaguilar/Projects/returns/returns/tracing.py:41 in `_get_trace`
/home/pabloaguilar/Projects/returns/returns/result.py:376 in `__init__`
/home/pabloaguilar/Projects/returns/returns/result.py:521 in `Failure`
/home/pabloaguilar/Projects/returns/example.py:33 in `my_func_one`
/home/pabloaguilar/Projects/returns/example.py:67 in `<module>`

call stack starting from third position

/home/pabloaguilar/Projects/returns/returns/result.py:521 in `Failure`
/home/pabloaguilar/Projects/returns/example.py:33 in `my_func_one`
/home/pabloaguilar/Projects/returns/example.py:67 in `<module>`

closes #409

@codecov
Copy link

codecov bot commented Jun 13, 2020

Codecov Report

Merging #422 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #422   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           47        48    +1     
  Lines         1192      1221   +29     
  Branches       154       154           
=========================================
+ Hits          1192      1221   +29     
Impacted Files Coverage Δ
returns/io.py 100.00% <100.00%> (ø)
returns/primitives/tracing.py 100.00% <100.00%> (ø)
returns/result.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf1018f...91cecbe. Read the comment docs.

@thepabloaguilar
Copy link
Member Author

Should I have add something to CHANGELOG.md?

returns/tracing.py Outdated Show resolved Hide resolved
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! Some polishing is required and we are ready to go!

docs/pages/tracing.rst Outdated Show resolved Hide resolved
docs/pages/tracing.rst Outdated Show resolved Hide resolved
docs/pages/tracing.rst Outdated Show resolved Hide resolved
returns/primitives/container.py Outdated Show resolved Hide resolved
returns/tracing.py Outdated Show resolved Hide resolved
returns/tracing.py Outdated Show resolved Hide resolved
@thepabloaguilar thepabloaguilar requested a review from sobolevn June 16, 2020 05:41
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

I also got an idea that we should add an opportunity to test traces with out pytest plugins.
This should help people to predict which paths the code will take.

But, I am not sure about an API. Do you have any ideas?

returns/tracing.py Outdated Show resolved Hide resolved
returns/tracing.py Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Show resolved Hide resolved
docs/pages/development.rst Show resolved Hide resolved
@thepabloaguilar
Copy link
Member Author

Awesome!

I also got an idea that we should add an opportunity to test traces with out pytest plugins.
This should help people to predict which paths the code will take.

But, I am not sure about an API. Do you have any ideas?

I didn't understand your idea, sorry!
Could you provide an example or more details?

@thepabloaguilar thepabloaguilar requested a review from sobolevn June 17, 2020 01:45
@sobolevn
Copy link
Member

I didn't understand your idea, sorry!
Could you provide an example or more details?

I will create a new issue for it.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like almost done!

I am pretty sure that this is the last round of review and we are going to merge this soon.
You have my gratitude, thanks a lot! 🥇

docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
docs/pages/development.rst Outdated Show resolved Hide resolved
returns/result.py Show resolved Hide resolved
returns/tracing.py Outdated Show resolved Hide resolved
returns/tracing.py Outdated Show resolved Hide resolved
@thepabloaguilar
Copy link
Member Author

Looks like almost done!

I am pretty sure that this is the last round of review and we are going to merge this soon.
You have my gratitude, thanks a lot!

Thanks man, it's a pleasure to contribute to returns!

@thepabloaguilar thepabloaguilar requested a review from sobolevn June 18, 2020 01:46
@sobolevn sobolevn merged commit 697479e into dry-python:master Jun 18, 2020
@sobolevn
Copy link
Member

Thanks again!

P.S. Can you please take a look at #258? I am researching possible design ideas right now. I would love to hear your thoughts.

@thepabloaguilar thepabloaguilar deleted the feature/refs-409 branch June 18, 2020 13:54
@thepabloaguilar
Copy link
Member Author

P.S. Can you please take a look at #258? I am researching possible design ideas right now. I would love to hear your thoughts.

Yes, I'll take a look asap!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Improve Failure tracibility
2 participants