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

Add --no-trace option, overrides pytrace=True for pytest.fail #6652

Closed

Conversation

FortStatement
Copy link

Hey, I just opened this issue to open a PR for the fork I'll be using for my automated exercise testing.
Hoping this change can be useful to others as well 😄

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

The idea/issue is nice, but I think it needs a better / more central method.
Note that _repr_failure_py is bad already (with regard to typing) to return a str here in some cases - this should not get extended. (btw: it also does not handle --full-trace here (blueyed@140d256)).

So I think the option in general makes sense, but should be handled diffrently maybe.

btw: have you tried --tb=line?

@blueyed
Copy link
Contributor

blueyed commented Feb 1, 2020

Another idea that comes to mind is enforcing pytrace=False in OutcomeException always when the option is enabled.

@FortStatement
Copy link
Author

That's where I started, but the session isn't available there in a clear manner.
The _repr_failure_py is a relatively nice place for it, but perhaps it should be organised a bit more nicely.

@nicoddemus
Copy link
Member

Note that _repr_failure_py is bad already (with regard to typing) to return a str here in some cases - this should not get extended.

Not sure I agree... I don't think the method is being extended; it used to return str in some instances, and continues to return str in an additional condition... is that what you mean by "extended"?

I agree that _repr_failure_py is a bit messy, but this is not making it worse than already is.

@blueyed
Copy link
Contributor

blueyed commented Feb 4, 2020

Note that _repr_failure_py is bad already (with regard to typing) to return a str here in some cases - this should not get extended.

Not sure I agree... I don't think the method is being extended; it used to return str in some instances, and continues to return str in an additional condition...

I've looked into making it not return str anymore, so this would make that a bit more difficult.

Ref: #3399

@GPery

btw: have you tried --tb=line?

@FortStatement
Copy link
Author

@GPery

btw: have you tried --tb=line?

I have not.
This will do nicely, thank you!

@nicoddemus
Copy link
Member

So I think --tb=line is sufficient and we can close this @GPery?

@FortStatement
Copy link
Author

No, I was mistaken on the last comment, --tb=line still outputs the file path, which doesn't work for me. --tb=none also wouldn't do, because it doesn't output the failure message. I'll keep using the fork, but I'm not sure if my use case is common enough that anyone would find it necessary.

I would appreciate the merge, it's a minor change that'll make things easier for me (keeping up to date with upstream).

@bluetech
Copy link
Member

My opinion is that this need is not common enough to deserve a configuration option, so I propose we close this PR. The issue can remain open in case someone comes up with another suggestion for satisfying this need.

@bluetech
Copy link
Member

Now that #7100 is merged, we have a better avenue for resolving this issue, as mentioned here: #7100 (review), so I'm closing this PR.

@bluetech bluetech closed this May 30, 2020
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.

4 participants