Skip to content

Conversation

diegomrsantos
Copy link
Contributor

@diegomrsantos diegomrsantos commented Oct 27, 2022

Motivation

When a test fails (either because of a bug in the code or because the developer intentionally caused the error to see if the test fails as expected) we should see the error that happened inside the test code, but currently, if an error also happens inside the teardown (potentially because of the previous error) the original error is swallowed and not shown.

This is a beginner's attempt to improve error handling.

unittest2.nim Outdated
Comment on lines 366 to 395
if stackTraces.len > 0:
echo stackTraces[i]
Copy link
Contributor Author

@diegomrsantos diegomrsantos Oct 27, 2022

Choose a reason for hiding this comment

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

This is not related to the fix, but IMHO it is more intuitive to see first the exception and then the stacktrace.

@diegomrsantos
Copy link
Contributor Author

Master is also failing with nim 1.7.3

arnetheduck
arnetheduck previously approved these changes Nov 7, 2022
jangko
jangko previously approved these changes Feb 14, 2023
@jangko
Copy link
Contributor

jangko commented Feb 14, 2023

somehow this patch breaks parallel test randomly

@diegomrsantos
Copy link
Contributor Author

diegomrsantos commented Apr 5, 2023

somehow this patch breaks parallel test randomly

@jangko Could you please suggest a simple way to reproduce it?

@jangko
Copy link
Contributor

jangko commented Apr 13, 2023

I only see it in the CI, not locally

@diegomrsantos
Copy link
Contributor Author

This job seems to be failing for a similar reason https://github.com/status-im/nim-unittest2/actions/runs/4204629692/jobs/7295560458. @Menduist

@arnetheduck
Copy link
Member

somehow this patch breaks parallel test randomly

I'm seeing the same breakage in another PR that cannot be the root cause - I suspect it's a bad test more than anything else

@diegomrsantos diegomrsantos force-pushed the handling-teardwon-exception branch from a22c5fa to d90f33e Compare September 5, 2023 12:29
@diegomrsantos
Copy link
Contributor Author

@arnetheduck could you please review the last version, not sure that's how the tests are supposed to look now.

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.

3 participants