www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Postmortem: Template unittests are bad & you shouldn't catch Error

reply Mathias LANG <geod24 gmail.com> writes:
Hi everyone,
As some of you may know, my company is developing a blockchain 
where the node is written solely in D 
(https://github.com/bpfkorea/agora).
One of the reason to pick D was the ease of prototyping and the 
C++ compatibility (we need to interface with modern C++ code, 
e.g. C++14 and 17).

Since tests matters, we have developed a testing framework which 
use(d) `std.concurrency` to simulate a network in memory (1 
thread == 1 node), which works *very well*.

However, there was a little gotcha: When an error happens, the 
thread would crash, and we would get a timeout in the main 
thread, and sometimes would not be able to print the logs. We use 
Ocean's Logger, which is derived from Tango's, which allow to 
essentially set the output, and in test mode we set it to a large 
static array acting as a circular buffer.

One of our developer was hit particularly hard by this while 
implementing a new feature, so added a bunch of `writeln`. After 
discussion with the team, the solution seemed obvious: Let's use 
`assertHandler` !

For those of you not familiar with it, here is `assertHandler`'s 
documentation: 
https://dlang.org/library/core/exception/assert_handler.html
It essentially allows to define a custom behavior on `assert`. It 
is slightly redundant with other facilities to do so, such as 
`-checkaction`, which allow to customize the behavior at the 
compiler level.

That's when things went bad. Since we wanted a core dump, we 
added an `abort` in our handler, expecting that it will only 
trigger when we have bugs. And our tests started to fail. And 
that's where the rant begins.

We got hit by this (reduced) stack trace:
```
[/usr/local/Cellar/ldc/1.23.0/include/dlang/ldc/std/typecons.d:2979] Assertion
thrown during test: Called `get' on null Nullable!int.
----------------
??:? nothrow void 
agora.test.Base.testAssertHandler(immutable(char)[], ulong, 
immutable(char)[]) [0x107807564]
??:? onAssertErrorMsg [0x10838ea21]
??:? _d_assert_msg [0x10838ee35]
??:? inout pure nothrow ref  property  nogc  safe inout(int) 
std.typecons.Nullable!(int).Nullable.get() [0x1079146d5]
??:? inout pure nothrow ref  property  nogc  safe inout(int) 
std.typecons.Nullable!(int).Nullable.get_() [0x107911958]
??:? pure nothrow  nogc  safe int 
std.typecons.Nullable!(immutable(char)[]).Nullable.__unittest_L2998_C1
_1().__dgliteral1() [0x10791193c]
??:? pure void 
std.exception.assertThrown!(core.exception.AssertError, 
int).assertThrown(lazy int, immutable(char)[], immutable(char)[], 
ulong) [0x10791177b]
??:? pure void 
std.typecons.Nullable!(immutable(char)[]).Nullable.__unittest_L2998_C13_1()
[0x1079116b8]
??:? agora.common.Serializer.__unittest [0x107bf97d8]
??:? void 
agora.test.Base.customModuleUnitTester().runTest(agora.test.Base.customModuleU
itTester().ModTest) [0x107807d46]
??:? core.runtime.UnitTestResult 
agora.test.Base.customModuleUnitTester() [0x10780712d]
```

The failing test is this: 
https://github.com/dlang/phobos/blob/8fe6a2762ae1b8b5ea906f15049a4373afbb45b5/std/typecons.d#L3000-L3015

There's so many things wrong here that I don't even know where to 
start.

First, why on god's green earth are we putting unittests with 
explicit template instantiation inside a *template* ? `unittest` 
inside templates are a terrible "feature", and I've never seen a 
correct usage of it, which would be a unittest that relies on the 
user-instantiated instance to test that its expectation still 
holds.
I'm pretty sure the rationale is "for documentation", and we 
should really re-think this, because those unittests are being 
run from *every single module that instantiate Nullable*. 
Actually, they are being run for *every instantiation of 
Nullable*.

Second, as the title of the post says, *do not catch Error*. 
Catching `Error` is a sin, a crime, and every catch of `Error` 
derivative should have you outraged. It is in direct violation of 
the specs. And obviously, it's also incompatible with scheme that 
do not throw an `Error`.

The code was introduced 5 years ago 
(https://github.com/dlang/phobos/pull/3114) and has been sitting 
there since then.

Another thing that contributed to our issue was that we didn't 
really knew where this instantiation came from. The module that 
we see in the stack trace, Serializer, has *no* reference to 
`Nullable`, and doesn't import `typecons`. It does import some 
other modules, so I'm conjecturing that the dependencies' 
unittests are executed first, but I'll take that up with the LDC 
team after I investigate.

In any case, there's two things I really wish we fixed:
- Either ban unittests in templates, deprecate them, or find a 
way to make this mistake hard to do;
- Deprecate the ability to catch `Error` in code. If it's not 
supposed to be caught, it shouldn't be so easy to do it;
Oct 21 2020
next sibling parent Kagamin <spam here.lot> writes:
Druntime catches exceptions on upper level, logs and aborts there.
Oct 22 2020
prev sibling next sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 10/22/20 12:04 AM, Mathias LANG wrote:
 Hi everyone,
 As some of you may know, my company is developing a blockchain where the 
 node is written solely in D (https://github.com/bpfkorea/agora).
 One of the reason to pick D was the ease of prototyping and the C++ 
 compatibility (we need to interface with modern C++ code, e.g. C++14 and 
 17).
 
 Since tests matters, we have developed a testing framework which use(d) 
 `std.concurrency` to simulate a network in memory (1 thread == 1 node), 
 which works *very well*.
 
 However, there was a little gotcha: When an error happens, the thread 
 would crash, and we would get a timeout in the main thread, and 
 sometimes would not be able to print the logs. We use Ocean's Logger, 
 which is derived from Tango's, which allow to essentially set the 
 output, and in test mode we set it to a large static array acting as a 
 circular buffer.
 
 One of our developer was hit particularly hard by this while 
 implementing a new feature, so added a bunch of `writeln`. After 
 discussion with the team, the solution seemed obvious: Let's use 
 `assertHandler` !
 
 For those of you not familiar with it, here is `assertHandler`'s 
 documentation: https://dlang.org/library/core/exception/assert_handler.html
 It essentially allows to define a custom behavior on `assert`. It is 
 slightly redundant with other facilities to do so, such as 
 `-checkaction`, which allow to customize the behavior at the compiler 
 level.
 
 That's when things went bad. Since we wanted a core dump, we added an 
 `abort` in our handler, expecting that it will only trigger when we have 
 bugs. And our tests started to fail. And that's where the rant begins.
 
 We got hit by this (reduced) stack trace:
 ```
 [/usr/local/Cellar/ldc/1.23.0/include/dlang/ldc/std/typecons.d:2979] 
 Assertion thrown during test: Called `get' on null Nullable!int.
 ----------------
 ??:? nothrow void agora.test.Base.testAssertHandler(immutable(char)[], 
 ulong, immutable(char)[]) [0x107807564]
 ??:? onAssertErrorMsg [0x10838ea21]
 ??:? _d_assert_msg [0x10838ee35]
 ??:? inout pure nothrow ref  property  nogc  safe inout(int) 
 std.typecons.Nullable!(int).Nullable.get() [0x1079146d5]
 ??:? inout pure nothrow ref  property  nogc  safe inout(int) 
 std.typecons.Nullable!(int).Nullable.get_() [0x107911958]
 ??:? pure nothrow  nogc  safe int 
 std.typecons.Nullable!(immutable(char)[]).Nullable.__unittest_L2998_C1
_1().__dgliteral1() 
 [0x10791193c]
 ??:? pure void std.exception.assertThrown!(core.exception.AssertError, 
 int).assertThrown(lazy int, immutable(char)[], immutable(char)[], ulong) 
 [0x10791177b]
 ??:? pure void 
 std.typecons.Nullable!(immutable(char)[]).Nullable.__unittest_L2998_C13_1() 
 [0x1079116b8]
 ??:? agora.common.Serializer.__unittest [0x107bf97d8]
 ??:? void 
 agora.test.Base.customModuleUnitTester().runTest(agora.test.Base.customModuleU
itTester().ModTest) 
 [0x107807d46]
 ??:? core.runtime.UnitTestResult 
 agora.test.Base.customModuleUnitTester() [0x10780712d]
 ```
 
 The failing test is this: 
 https://github.com/dlang/phobos/blob/8fe6a2762ae1b8b5ea906f15049a4373afbb45b5/std/ty
econs.d#L3000-L3015 
 
 
 There's so many things wrong here that I don't even know where to start.
 
 First, why on god's green earth are we putting unittests with explicit 
 template instantiation inside a *template* ? `unittest` inside templates 
 are a terrible "feature", and I've never seen a correct usage of it, 
 which would be a unittest that relies on the user-instantiated instance 
 to test that its expectation still holds.
I used it in dcollections quite a bit as a way to ensure all integer based types were tested. The advantage here is, I wanted to run the same tests on every integer type. But I wanted the tests to be close to the functions they were testing. But I think this can be done a different way, and it is *really* hard to get this right. Plus, they are going to be run when you instantiate them elsewhere, which is a problem.
 I'm pretty sure the rationale is "for documentation", and we should 
 really re-think this, because those unittests are being run from *every 
 single module that instantiate Nullable*. Actually, they are being run 
 for *every instantiation of Nullable*.
Yeah, that is bad. This unittest should be moved outside the template.
 
 Second, as the title of the post says, *do not catch Error*. Catching 
 `Error` is a sin, a crime, and every catch of `Error` derivative should 
 have you outraged. It is in direct violation of the specs. And 
 obviously, it's also incompatible with scheme that do not throw an `Error`.
I disagree with this. How would you test that the assert error is thrown? It seems that your specific case is being hampered by this, but can't you just look at the parameters to see whether you should handle it or not? Like check the file/line and do the normal thing if it's from phobos?
 Another thing that contributed to our issue was that we didn't really 
 knew where this instantiation came from. The module that we see in the 
 stack trace, Serializer, has *no* reference to `Nullable`, and doesn't 
 import `typecons`. It does import some other modules, so I'm 
 conjecturing that the dependencies' unittests are executed first, but 
 I'll take that up with the LDC team after I investigate.
 
 In any case, there's two things I really wish we fixed:
 - Either ban unittests in templates, deprecate them, or find a way to 
 make this mistake hard to do;
What we need is a way to make a static unittest that serves to provide documentation, but is run as if it were outside the template.
 - Deprecate the ability to catch `Error` in code. If it's not supposed 
 to be caught, it shouldn't be so easy to do it;
Disagree. This use case you have is very obscure, and it's much less obscure to verify an Error is thrown in a unittest. -Steve
Oct 22 2020
parent reply Mathias LANG <geod24 gmail.com> writes:
On Thursday, 22 October 2020 at 14:36:11 UTC, Steven 
Schveighoffer wrote:
 On 10/22/20 12:04 AM, Mathias LANG wrote:
 
 First, why on god's green earth are we putting unittests with 
 explicit template instantiation inside a *template* ? 
 `unittest` inside templates are a terrible "feature", and I've 
 never seen a correct usage of it, which would be a unittest 
 that relies on the user-instantiated instance to test that its 
 expectation still holds.
I used it in dcollections quite a bit as a way to ensure all integer based types were tested. The advantage here is, I wanted to run the same tests on every integer type. But I wanted the tests to be close to the functions they were testing. But I think this can be done a different way, and it is *really* hard to get this right. Plus, they are going to be run when you instantiate them elsewhere, which is a problem.
The point was, if the unittest does not depend on the template parameters, it shouldn't be in the template. So the following is a correct use of the feature: ``` struct Nullable (T) { unittest { Nullable inst; assert(inst.isNull()); } } ``` While this is not: ``` struct Nullable (T) { unittest { Nullable!int inst; assert(inst.isNull()); } } ``` Because the first one will run for the user instantiation while the second one is independent on the user instantiation (but will run anyway).
 
 Second, as the title of the post says, *do not catch Error*. 
 Catching `Error` is a sin, a crime, and every catch of `Error` 
 derivative should have you outraged. It is in direct violation 
 of the specs. And obviously, it's also incompatible with 
 scheme that do not throw an `Error`.
I disagree with this. How would you test that the assert error is thrown? It seems that your specific case is being hampered by this, but can't you just look at the parameters to see whether you should handle it or not? Like check the file/line and do the normal thing if it's from phobos?
You do not test for assert error. Assert errors are logic errors, and if they get triggered, all bets are off. Of course, one might find this impractical, because `get` cannot be tested this way. Perhaps that's a sign that `Nullable` should use exceptions, and not `assert`, for this. Regarding the workaround: We currently do this, however, testing for it in a cross-platform and generic way is non-trivial. We can do `endsWith` but we have to account for different paths. And there's no guarantee that we won't have false-positive (e.g. ourcustomlib.std.typecons).
 In any case, there's two things I really wish we fixed:
 - Either ban unittests in templates, deprecate them, or find a 
 way to make this mistake hard to do;
What we need is a way to make a static unittest that serves to provide documentation, but is run as if it were outside the template.
That's something that Andrej Mitrovic brought up during the following discussion. While it would fix this specific instance, the default would still be bad, so I'm not really convinced.
 - Deprecate the ability to catch `Error` in code. If it's not 
 supposed to be caught, it shouldn't be so easy to do it;
Disagree. This use case you have is very obscure, and it's much less obscure to verify an Error is thrown in a unittest. -Steve
You disagree with the spec then. The spec says that the stack might not be unwound if an `Error` is thrown. Walter even raised a PR to actually do the optimization the spec promises. In his words:
 This PR is still the right thing to do. Attempting to unwind 
 when an Error is thrown is a bad idea, and leads to undefined 
 behavior. Essentially, code which relies on such unwinding 
 needs to be fixed.
Source: https://github.com/dlang/dmd/pull/6896#issuecomment-362855793
Oct 22 2020
next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 22.10.20 18:05, Mathias LANG wrote:

 
 You disagree with the spec then. The spec says that the stack might not 
 be unwound if an `Error` is thrown. Walter even raised a PR to actually 
 do the optimization the spec promises. In his words:
 
 This PR is still the right thing to do. Attempting to unwind when an 
 Error is thrown is a bad idea, and leads to undefined behavior. 
 Essentially, code which relies on such unwinding needs to be fixed.
Source: https://github.com/dlang/dmd/pull/6896#issuecomment-362855793
It's not that simple. The compiler itself catches AssertError when checking preconditions.
Oct 22 2020
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 22.10.20 18:26, Timon Gehr wrote:
 On 22.10.20 18:05, Mathias LANG wrote:

 You disagree with the spec then. The spec says that the stack might 
 not be unwound if an `Error` is thrown. Walter even raised a PR to 
 actually do the optimization the spec promises. In his words:

 This PR is still the right thing to do. Attempting to unwind when an 
 Error is thrown is a bad idea, and leads to undefined behavior. 
 Essentially, code which relies on such unwinding needs to be fixed.
Source: https://github.com/dlang/dmd/pull/6896#issuecomment-362855793
It's not that simple. The compiler itself catches AssertError when checking preconditions.
Also, code that "relies on such unwinding" can be annotated safe and the compiler won't complain. The issue is not that people disagree with the specification, it's that the specification disagrees with itself.
Oct 22 2020
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 22.10.20 18:28, Timon Gehr wrote:
 
 Also, code that "relies on such unwinding" can be annotated  safe and 
 the compiler won't complain.
Actually, it will complain. Sorry for the noise, should have double-checked before sending the post. However, in contracts do rely on catching AssertError, and they are allowed in safe code: --- void bar() safe{ import std.stdio; writeln("throwing an AssertError"); assert(0); } void main() safe{ class C{ void foo() safe in{ bar(); }do{} } class D:C{ override void foo() safe in{}do{} } auto d=new D; d.foo(); } --- Prints: throwing an AssertError The program does not abort. If catching an AssertError is UB, why does the language do it in safe code?
Oct 22 2020
prev sibling next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Oct 22, 2020 at 04:05:51PM +0000, Mathias LANG via Digitalmars-d wrote:
 On Thursday, 22 October 2020 at 14:36:11 UTC, Steven Schveighoffer wrote:
 On 10/22/20 12:04 AM, Mathias LANG wrote:
[...]
 Second, as the title of the post says, *do not catch Error*.
[...]
 I disagree with this. How would you test that the assert error is
 thrown? It seems that your specific case is being hampered by this,
 but can't you just look at the parameters to see whether you should
 handle it or not? Like check the file/line and do the normal thing
 if it's from phobos?
You do not test for assert error. Assert errors are logic errors, and if they get triggered, all bets are off. Of course, one might find this impractical, because `get` cannot be tested this way. Perhaps that's a sign that `Nullable` should use exceptions, and not `assert`, for this.
[...] +1. Catching Errors is an anti-pattern and a code smell. The usual argument is, I want to test that my asserts are working. The problem with that is that it quickly falls into the paradox of who watches the watchers. The next thing you know, I'm asking for a way to test that my code that catches asserts are actually catching asserts. Next year I'll be asking for code to test my code that tests code that catches asserts. It never ends. The point is, if your asserts are so complex you have to write unittests for them, then there's something fundamentally wrong with them, and they should be rewritten. Like with exceptions, as Mathias says. It's a different story altogether if druntime uses catching of Errors as a low-level mechanism to implement certain language semantics. I know contracts in a class hierarchy are implemented that way for certain reasons, for example. But that's stuff under the hood, that should not be done in user code, ever. T -- Obviously, some things aren't very obvious.
Oct 22 2020
next sibling parent reply Kagamin <spam here.lot> writes:
On Thursday, 22 October 2020 at 16:30:57 UTC, H. S. Teoh wrote:
 The problem with that is that it quickly falls into the paradox 
 of who watches the watchers.  The next thing you know, I'm 
 asking for a way to test that my code that catches asserts are 
 actually catching asserts.  Next year I'll be asking for code 
 to test my code that tests code that catches asserts. It never 
 ends.
Paradoxes don't exist. Tests are watchers, and practice shows their presence is better than their absence.
Oct 22 2020
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Oct 22, 2020 at 07:31:39PM +0000, Kagamin via Digitalmars-d wrote:
 On Thursday, 22 October 2020 at 16:30:57 UTC, H. S. Teoh wrote:
 The problem with that is that it quickly falls into the paradox of
 who watches the watchers.  The next thing you know, I'm asking for a
 way to test that my code that catches asserts are actually catching
 asserts.  Next year I'll be asking for code to test my code that
 tests code that catches asserts. It never ends.
Paradoxes don't exist. Tests are watchers, and practice shows their presence is better than their absence.
I agree tests are watchers. But it starts a bit ridiculous when you need to test your tests... T -- Prosperity breeds contempt, and poverty breeds consent. -- Suck.com
Oct 22 2020
next sibling parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 10/22/20 4:54 PM, H. S. Teoh wrote:
 On Thu, Oct 22, 2020 at 07:31:39PM +0000, Kagamin via Digitalmars-d wrote:
 On Thursday, 22 October 2020 at 16:30:57 UTC, H. S. Teoh wrote:
 The problem with that is that it quickly falls into the paradox of
 who watches the watchers.  The next thing you know, I'm asking for a
 way to test that my code that catches asserts are actually catching
 asserts.  Next year I'll be asking for code to test my code that
 tests code that catches asserts. It never ends.
Paradoxes don't exist. Tests are watchers, and practice shows their presence is better than their absence.
I agree tests are watchers. But it starts a bit ridiculous when you need to test your tests...
Testing that an error occurs in an error situation is not ridiculous. Repeating the question that still isn't answered, I want to make sure when someone accesses a Nullable that is null using the `get` function, an error is thrown. How do I check for this other than trying it, and examining the error? The fact that it throws this specific error in this situation is part of the API, I want to test that API. How do I do it? If this were an error code being returned we aren't having this conversation, it would be obviously acceptable. It needs to be acceptable here as well. -Steve
Oct 23 2020
prev sibling next sibling parent Dukc <ajieskola gmail.com> writes:
On Thursday, 22 October 2020 at 20:54:52 UTC, H. S. Teoh wrote:
 I agree tests are watchers.  But it starts a bit ridiculous 
 when you need to test your tests...
It might not be. In my opinion, the answer to "who watches the watchmen?" is "a manual test". If you automate a test, you should test the test yourself. If you automate testing the test, you should test the test-tester and so on. What is the point then in unit tests? I believe it is in reducing the amount of manual testing. You will not have to change a good unit test nearly as often as you have to change the code it is testing. If it does not change, it does not need rechecking. The assertions within the function usually don't share this property. They may rely on the internal logic of the function, and thus get changed with it. Manually testing them sucks just as much as relying on manual tests in general. Contracts might be better in this regard though, as long as the function is either public or widely used internally, so that it's interface and intended functionality don't change often.
Oct 23 2020
prev sibling parent Kagamin <spam here.lot> writes:
On Thursday, 22 October 2020 at 20:54:52 UTC, H. S. Teoh wrote:
 I agree tests are watchers.  But it starts a bit ridiculous 
 when you need to test your tests...
They have tests: https://github.com/atilaneves/unit-threaded/blob/master/tests/unit_threaded/ut/should.d
Oct 23 2020
prev sibling parent Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 16:30:57 UTC, H. S. Teoh wrote:

 It's a different story altogether if druntime uses catching of 
 Errors as a low-level mechanism to implement certain language 
 semantics.  I know contracts in a class hierarchy are 
 implemented that way for certain reasons, for example.  But 
 that's stuff under the hood, that should not be done in user 
 code, ever.
And who would you then implement a new unit test runner without catching `AssertError`? Make it even worse than the built-in one and abort the whole program after a single failed test? The program is still in a defined state when an assertion inside a `unittest` block fails because the assertion is not assumed to hold [1]. [1] https://dlang.org/spec/unittest.html -- /Jacob Carlborg
Oct 23 2020
prev sibling next sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 10/22/20 12:05 PM, Mathias LANG wrote:
 On Thursday, 22 October 2020 at 14:36:11 UTC, Steven Schveighoffer wrote:
 On 10/22/20 12:04 AM, Mathias LANG wrote:
 First, why on god's green earth are we putting unittests with 
 explicit template instantiation inside a *template* ? `unittest` 
 inside templates are a terrible "feature", and I've never seen a 
 correct usage of it, which would be a unittest that relies on the 
 user-instantiated instance to test that its expectation still holds.
I used it in dcollections quite a bit as a way to ensure all integer based types were tested. The advantage here is, I wanted to run the same tests on every integer type. But I wanted the tests to be close to the functions they were testing. But I think this can be done a different way, and it is *really* hard to get this right. Plus, they are going to be run when you instantiate them elsewhere, which is a problem.
The point was, if the unittest does not depend on the template parameters, it shouldn't be in the template. So the following is a correct use of the feature: ``` struct Nullable (T) {     unittest     {         Nullable inst;         assert(inst.isNull());     } } ``` While this is not: ``` struct Nullable (T) {     unittest     {         Nullable!int inst;         assert(inst.isNull());     } } ``` Because the first one will run for the user instantiation while the second one is independent on the user instantiation (but will run anyway).
Yes, I know. It's exactly what I did: https://github.com/schveiguy/dcollections/blob/master/dcollections/TreeMap.d#L123 https://github.com/schveiguy/dcollections/blob/master/dcollections/TreeMap.d#L253
 
 Second, as the title of the post says, *do not catch Error*. Catching 
 `Error` is a sin, a crime, and every catch of `Error` derivative 
 should have you outraged. It is in direct violation of the specs. And 
 obviously, it's also incompatible with scheme that do not throw an 
 `Error`.
I disagree with this. How would you test that the assert error is thrown? It seems that your specific case is being hampered by this, but can't you just look at the parameters to see whether you should handle it or not? Like check the file/line and do the normal thing if it's from phobos?
You do not test for assert error. Assert errors are logic errors, and if they get triggered, all bets are off. Of course, one might find this impractical, because `get` cannot be tested this way. Perhaps that's a sign that `Nullable` should use exceptions, and not `assert`, for this.
You did not answer the question though, the unittest is making sure that an assert error is thrown when you attempt to get a null value. How else do you test for this? Nullable using exceptions means it can't be used in nothrow code. This argument has been had countless times for many different discussions. I thought AssertErrors during unit tests are allowed to be caught by the unit test framework? Isn't assert the main tool to test things? If you aren't supposed to catch them ever, how does unittesting work?
 Regarding the workaround: We currently do this, however, testing for it 
 in a cross-platform and generic way is non-trivial. We can do `endsWith` 
 but we have to account for different paths. And there's no guarantee 
 that we won't have false-positive (e.g. ourcustomlib.std.typecons).
Isn't this for finding a bug though? I didn't think you were going to enable the custom assert handler for production. So who cares if it's not neat and clean, get it to work, find the bug, and move on.
 In any case, there's two things I really wish we fixed:
 - Either ban unittests in templates, deprecate them, or find a way to 
 make this mistake hard to do;
What we need is a way to make a static unittest that serves to provide documentation, but is run as if it were outside the template.
That's something that Andrej Mitrovic brought up during the following discussion. While it would fix this specific instance, the default would still be bad, so I'm not really convinced.
It needs to happen. There's no reason I shouldn't be able to use documented unittests in templates. Another thing I've advocated for strongly is to not run imported unittests ever, even in templates.
 
 - Deprecate the ability to catch `Error` in code. If it's not 
 supposed to be caught, it shouldn't be so easy to do it;
Disagree. This use case you have is very obscure, and it's much less obscure to verify an Error is thrown in a unittest.
You disagree with the spec then. The spec says that the stack might not be unwound if an `Error` is thrown.
The stack may not be unwound. That doesn't mean the program is in an invalid state, especially if you control all the code you are testing. For sure, it should be almost never done. But almost never is not never.
 Walter even raised a PR to actually 
 do the optimization the spec promises. In his words:
 This PR is still the right thing to do. Attempting to unwind when an 
 Error is thrown is a bad idea, and leads to undefined behavior. 
 Essentially, code which relies on such unwinding needs to be fixed.
Source: https://github.com/dlang/dmd/pull/6896#issuecomment-362855793
That has no relation to this. There are no unwinding tasks. -Steve
Oct 22 2020
next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Oct 22, 2020 at 03:18:03PM -0400, Steven Schveighoffer via
Digitalmars-d wrote:
[...]
 Another thing I've advocated for strongly is to not run imported
 unittests ever, even in templates.
[...] +1. This has necessitated the silly StdUnittest hack in Phobos, and AFAIK no other D library has used a similar hack, which means that every single time somebody imports an 3rd party library and compiles with -unittest, the library's unittests get run all over again for the millionth time. It's useless weight and redundant work, and should be eliminated. T -- Democracy: The triumph of popularity over principle. -- C.Bond
Oct 22 2020
prev sibling parent Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 19:18:03 UTC, Steven 
Schveighoffer wrote:

 I thought AssertErrors during unit tests are allowed to be 
 caught by the unit test framework?
Technically the spec doesn't say anything about that. What it does say is that if an assertion fails inside a `unittest` block the program is still in a valid state. I'm not sure that means you can catch `AssertError` or not. It's also not clear if it only applies to `assert` used directly inside a `unittest` block or any code reachable through a `unittest` block.
 Isn't assert the main tool to test things?
Yes.
 If you aren't supposed to catch them ever, how does unittesting 
 work?
Yes, exactly. If we should really follow that to the letter (which I don't think we should) then the program would abort as soon as a test failed. Then you would have to fix that failing test to see the rest of the failing tests.
 The stack may not be unwound. That doesn't mean the program is 
 in an invalid state, especially if you control all the code you 
 are testing.
Exactly, see above. [1] https://dlang.org/spec/unittest.html -- /Jacob Carlborg
Oct 23 2020
prev sibling parent reply Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 16:05:51 UTC, Mathias LANG wrote:

 You do not test for assert error. Assert errors are logic 
 errors, and if they get triggered, all bets are off.

 You disagree with the spec then. The spec says that the stack 
 might not be unwound if an `Error` is thrown.
It's not that simple, you need to read the whole spec ;). The spec for unit tests has an exception: "Individual tests are specified in the unit test using AssertExpressions. Unlike AssertExpressions used elsewhere, the assert is not assumed to hold, and upon assert failure the program is still in a defined state." [1]. Although, it's not clear if that only applies to `assert` directly in the `unittest` block or any assert reachable through the `unittest` block. Regardless of what's correct or not, I think there must be a way to have unit tests keep running after a failed test. Otherwise D will have very poor experience for unit test frameworks. It's already bad enough with the default unit test runner that aborts the rest of the unit tests inside a module after a failed test (it will continue with other modules). [1] https://dlang.org/spec/unittest.html -- /Jacob Carlborg
Oct 23 2020
next sibling parent Mathias LANG <geod24 gmail.com> writes:
On Friday, 23 October 2020 at 09:03:43 UTC, Jacob Carlborg wrote:
 It's not that simple, you need to read the whole spec ;). The 
 spec for unit tests has an exception:

 "Individual tests are specified in the unit test using 
 AssertExpressions. Unlike AssertExpressions used elsewhere, the 
 assert is not assumed to hold, and upon assert failure the 
 program is still in a defined state." [1].

 Although, it's not clear if that only applies to `assert` 
 directly in the `unittest` block or any assert reachable 
 through the `unittest` block.
I'm aware of this, and I think the spec is pretty clear. "Unlike `AssertExpression` used elsewhere" means that this only applies to `assert` inside unittest, not those transitively reachable. Note that a compiler could one day decide to rewrite those to exceptions to be more consistent with the specs.
 Regardless of what's correct or not, I think there must be a 
 way to have unit tests keep running after a failed test. 
 Otherwise D will have very poor experience for unit test 
 frameworks. It's already bad enough with the default unit test 
 runner that aborts the rest of the unit tests inside a module 
 after a failed test (it will continue with other modules).

 [1] https://dlang.org/spec/unittest.html

 --
 /Jacob Carlborg
A compiler rewrite would do that.
Oct 23 2020
prev sibling parent Kagamin <spam here.lot> writes:
On Friday, 23 October 2020 at 09:03:43 UTC, Jacob Carlborg wrote:
 Regardless of what's correct or not, I think there must be a 
 way to have unit tests keep running after a failed test. 
 Otherwise D will have very poor experience for unit test 
 frameworks. It's already bad enough with the default unit test 
 runner that aborts the rest of the unit tests inside a module 
 after a failed test (it will continue with other modules).
FWIW ldc performs unwinding optimizations only in release compilation mode and not in others.
Oct 23 2020
prev sibling next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Oct 22, 2020 at 04:04:26AM +0000, Mathias LANG via Digitalmars-d wrote:
[...]
 First, why on god's green earth are we putting unittests with explicit
 template instantiation inside a *template* ? `unittest` inside
 templates are a terrible "feature", and I've never seen a correct
 usage of it, which would be a unittest that relies on the
 user-instantiated instance to test that its expectation still holds.

 I'm pretty sure the rationale is "for documentation", and we should
 really re-think this, because those unittests are being run from
 *every single module that instantiate Nullable*. Actually, they are
 being run for *every instantiation of Nullable*.
[...] This is a known issue that has been brought up many times in the past. Basically: 1) A unittest block inside a template is duplicated for *every* instantiation of the template. To alleviate this, you'd need to do something like this: template MyTemplate(Args...) { auto myFunc(...) { ... } static if (Args == ...) unittest { ... } } This is not a fool-proof solution, however, because if that specific combination of Args isn't actually instantiated, the unittest is silently ignored (even if it would have triggered a failure due to some bug). To alleviate this, you'd have to insert something like this outside the template body: // Make sure unittests get instantiated. version(unittest) alias _ = MyTemplate!(...); Which is, of course, extremely ugly. And definitely not something newcomers would think of. Another way to solve this is to move the unittest outside the template body. However: 2) Ddoc's unittests require unittest blocks to be attached to the symbol they're documenting. So you pretty much have no choice, unless you use Phobos StdDdoc hack (ugh): struct PhobosTemplate { auto phobosFunc(...) { ... } version(StdDoc) // ugh /// unittest { ... } } Jonathan Davis & myself have proposed in the past an enhancement to solve this problem: have some way of marking a unittest block inside a template as single-instance only. Something like this: template MyTemplate(T) { auto myFunc(...) { ... } /// This is instantiated once per template instantiation unittest { pragma(msg, T.stringof); } /// This is instantiated only once, ever /// (The use of 'static' is hypothetical syntax, it can /// be anything else that marks the unittest as /// single-instance) static unittest { //pragma(msg, T.stringof); // Error: cannot reference T here // Have to explicitly instantiate the template // with the arguments you want to test for alias U = MyTemplate!int; } } However, so far no action has been taken on this front besides the proposal. T -- "Maybe" is a strange word. When mom or dad says it it means "yes", but when my big brothers say it it means "no"! -- PJ jr.
Oct 22 2020
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 22 October 2020 at 16:18:14 UTC, H. S. Teoh wrote:
 2) Ddoc's unittests require unittest blocks to be attached to 
 the symbol they're documenting.
The whole unittest as example thing was a mistake anyway. It doesn't actually work at all (stuff can pass the test without being a usable example due to scope contexts) and encourages ugly, unusable examples because they are run as automatic tests instead of you know, actually being written as examples. If I was in charge of it, the documented ones would have to be like static, in that they don't depend on *any* context - the compiler treats each block like its own pseudo-module and you must import everything you use again (this guarantees you only use public things and must demo a complete example) - and allow them to be decoupled. In adrdox, I let you tag them with an ID, then embed by id in the prose to insert the example where you want it. Maybe ddoc could do that too so you have some freedom to move it around even if you reject the static paragraph. (or y'all ditch horrible ddoc and come to good docs. the water's fine.)
Oct 22 2020
next sibling parent reply Mathias LANG <geod24 gmail.com> writes:
On Thursday, 22 October 2020 at 16:34:14 UTC, Adam D. Ruppe wrote:
 On Thursday, 22 October 2020 at 16:18:14 UTC, H. S. Teoh wrote:
 2) Ddoc's unittests require unittest blocks to be attached to 
 the symbol they're documenting.
The whole unittest as example thing was a mistake anyway. It doesn't actually work at all (stuff can pass the test without being a usable example due to scope contexts) and encourages ugly, unusable examples because they are run as automatic tests instead of you know, actually being written as examples. If I was in charge of it, the documented ones would have to be like static, in that they don't depend on *any* context - the compiler treats each block like its own pseudo-module and you must import everything you use again (this guarantees you only use public things and must demo a complete example) - and allow them to be decoupled. In adrdox, I let you tag them with an ID, then embed by id in the prose to insert the example where you want it. Maybe ddoc could do that too so you have some freedom to move it around even if you reject the static paragraph. (or y'all ditch horrible ddoc and come to good docs. the water's fine.)
Two pretty great idea. Do you have a bugzilla entry for them by any chance ? I don't agree with the sentiment that documented unittests are a mistake though: they are a step up from documentation examples, because you are at least guaranteed they compile and pass in the right context. Examples in documentation might not even compile from day 1 because of a typo, or fail to compile when a symbol / language feature is deprecated or removed. But making them context-independent is definitely an idea worth exploring.
Oct 22 2020
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 22 October 2020 at 17:14:54 UTC, Mathias LANG wrote:
 Do you have a bugzilla entry for them by any chance ?
I don't...
 I don't agree with the sentiment that documented unittests are 
 a mistake though: they are a step up from documentation 
 examples, because you are at least guaranteed they compile and 
 pass in the right context.
That's pretty easy to do outside the language though: extracting code from the generated documentation is trivial and then you can run them through the compiler externally. Of course that is an extra step so the unittest does have the advantage of being obvious and built in, but the official website could compile them as part of its existing auto test. And it can do it using the same live environment end users actually try, so same imports, same compiler version present on the "try it now" website, etc., for a more accurate test.
Oct 22 2020
parent reply Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 19:34:45 UTC, Adam D. Ruppe wrote:

 Of course that is an extra step so the unittest does have the 
 advantage of being obvious and built in, but the official 
 website could compile them as part of its existing auto test. 
 And it can do it using the same live environment end users 
 actually try, so same imports, same compiler version present on 
 the "try it now" website, etc., for a more accurate test.
It needs to work for every project, not just Phobos which has the "try it now". -- /Jacob Carlborg
Oct 23 2020
parent Adam D. Ruppe <destructionator gmail.com> writes:
On Friday, 23 October 2020 at 09:19:01 UTC, Jacob Carlborg wrote:
 It needs to work for every project, not just Phobos which has 
 the "try it now".
All projects can extract code from generated files and compile them. They can, but don't have to use the online thing, you might just compile the extracted files locally.
Oct 23 2020
prev sibling parent Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On Thursday, 22 October 2020 at 16:34:14 UTC, Adam D. Ruppe wrote:
 and encourages ugly, unusable examples because they are run as 
 automatic tests instead of you know, actually being written as 
 examples.
Sorry but that's not a problem with documented unittests. They should be written like so: /// struct S { } /// unittest { // show example of *simple* usage that is generated in the documentation } // NOT generated in the documentation, and can be heavy-duty and have contexts unittest { } The responsibility of writing good documentation unittests lies solely on the person introducing the new functionality (and the reviewers!). If people write terrible examples in `///` unittests then they write terrible documentation regardless. You can add the requirement for the need to import into the module if you want (and this is a good idea, maybe we could abuse static for this), but in the end those same people will still abuse the feature. They'll add the necessary import, and still write terrible documentation examples. Can't fix bad documentation unless there's someone willing to review and point it out.
Oct 23 2020
prev sibling next sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Thursday, 22 October 2020 at 16:18:14 UTC, H. S. Teoh wrote:
 2) Ddoc's unittests require unittest blocks to be attached to 
 the symbol they're documenting. So you pretty much have no 
 choice, unless you use Phobos StdDdoc hack (ugh):
It seems like the obvious solution is to remove this limitation. For example, maybe the documentation generator should allow you to attach a unittest to any symbol in the same module, regardless of where it appears in the code: /// Documents: Nullable.get unittest { /* etc. */ }
Oct 22 2020
next sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 10/22/20 12:39 PM, Paul Backus wrote:
 On Thursday, 22 October 2020 at 16:18:14 UTC, H. S. Teoh wrote:
 2) Ddoc's unittests require unittest blocks to be attached to the 
 symbol they're documenting. So you pretty much have no choice, unless 
 you use Phobos StdDdoc hack (ugh):
It seems like the obvious solution is to remove this limitation. For example, maybe the documentation generator should allow you to attach a unittest to any symbol in the same module, regardless of where it appears in the code: /// Documents: Nullable.get unittest {     /* etc. */ }
This would be sufficient! -Steve
Oct 22 2020
parent Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 22 October 2020 at 19:19:18 UTC, Steven 
Schveighoffer wrote:
 On 10/22/20 12:39 PM, Paul Backus wrote:
 /// Documents: Nullable.get
This would be sufficient!
I don't hate the syntax either, I think I'll steal it for adrdox...
Oct 22 2020
prev sibling parent Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 16:39:48 UTC, Paul Backus wrote:

 It seems like the obvious solution is to remove this 
 limitation. For example, maybe the documentation generator 
 should allow you to attach a unittest to any symbol in the same 
 module, regardless of where it appears in the code:

 /// Documents: Nullable.get
 unittest
 {
     /* etc. */
 }
I like that. -- /Jacob Carlborg
Oct 23 2020
prev sibling parent Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 16:18:14 UTC, H. S. Teoh wrote:

 	template MyTemplate(T)
 	{
 		auto myFunc(...) { ... }

 		/// This is instantiated once per template instantiation
 		unittest {
 			pragma(msg, T.stringof);
 		}

 		/// This is instantiated only once, ever
 		/// (The use of 'static' is hypothetical syntax, it can
 		/// be anything else that marks the unittest as
 		/// single-instance)
 		static unittest {
 			//pragma(msg, T.stringof); // Error: cannot reference T here

 			// Have to explicitly instantiate the template
 			// with the arguments you want to test for
 			alias U = MyTemplate!int;
 		}
 	}
Might be a bit confusing when the unit test is inside a templated class. Because the unit test function is already static, relative to the class. -- /Jacob Carlborg
Oct 23 2020
prev sibling next sibling parent reply Meta <jared771 gmail.com> writes:
On Thursday, 22 October 2020 at 04:04:26 UTC, Mathias LANG wrote:
<snip>
It just occurred to me that putting `version(none)` on those 
tests *might* disable them while also ensuring that the tests 
show up in the documentation (which was the idea behind putting 
them in there), but I'm not sure about that. I know that 
something like `static if (false) { declarations... }` will omit 
them.
Oct 22 2020
next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Thu, Oct 22, 2020 at 04:19:30PM +0000, Meta via Digitalmars-d wrote:
 On Thursday, 22 October 2020 at 04:04:26 UTC, Mathias LANG wrote:
 <snip>
 It just occurred to me that putting `version(none)` on those tests
 *might* disable them while also ensuring that the tests show up in the
 documentation (which was the idea behind putting them in there), but
 I'm not sure about that. I know that something like `static if (false)
 { declarations... }` will omit them.
I'm pretty sure ddocs on version(none) symbols are skipped. So this won't work. T -- "If you're arguing, you're losing." -- Mike Thomas
Oct 22 2020
prev sibling parent Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 22 October 2020 at 16:19:30 UTC, Meta wrote:
 It just occurred to me that putting `version(none)` on those 
 tests *might* disable them while also ensuring that the tests 
 show up in the documentation
You should just put the examples in the doc comment at that point. (which is really better anyway, that's the way it should have been done from the beginning!!!)
Oct 22 2020
prev sibling next sibling parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 10/21/20 9:04 PM, Mathias LANG wrote:

 *do not catch Error*. Catching
 `Error` is a sin, a crime, and every catch of `Error` derivative should
 have you outraged. It is in direct violation of the specs. And
 obviously, it's also incompatible with scheme that do not throw an 
`Error`. That's true but I will recommend catching Error in both of my upcoming DConf Online presentations. :) 1) Thread entry functions should catch and report any Throwable. Otherwise, main thread will see only a timeout and stack trace of the worker will lost. 2) extern(C) D library functions must catch Throwable and return non-zero error code because the caller is highly likely not going to be able to handle the situation. Even when our library function is called from D, meaning that it can handle D exceptions, we cannot be sure that we are in the main thread. (Can we detect that?). If I'm not mistaken, only the main thread dumps stack trace for Error; so, extern(C) function catching and reporting is essential. Is my thinking correct? Ali
Oct 22 2020
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Thursday, 22 October 2020 at 20:23:49 UTC, Ali Çehreli wrote:
 On 10/21/20 9:04 PM, Mathias LANG wrote:

 *do not catch Error*. Catching
 `Error` is a sin, a crime, and every catch of `Error`
derivative should
 have you outraged. It is in direct violation of the specs. And
 obviously, it's also incompatible with scheme that do not
throw an `Error`. That's true but I will recommend catching Error in both of my upcoming DConf Online presentations. :) 1) Thread entry functions should catch and report any Throwable. Otherwise, main thread will see only a timeout and stack trace of the worker will lost.
Once an Error has been thrown, the program is in an invalid state, and anything you do *on any thread* has undefined behavior. The only thing you can do safely is terminate the entire process. Stack traces can be retrieved after the fact from a core dump.
Oct 22 2020
next sibling parent =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 10/22/20 1:33 PM, Paul Backus wrote:

 Once an Error has been thrown, the program is in an invalid state, and
 anything you do *on any thread* has undefined behavior. The only thing
 you can do safely is terminate the entire process. Stack traces can be
 retrieved after the fact from a core dump.
Which means: 1) Must still catch Error in a worker thread to be able to abort the whole program. Otherwise no other thread is aware. 2) Is the default behavior of D runtime dumping stack trace correct? On the one hand, it attempts to do something, which should not be attempted. On the other hand, it does report after the program code has stopped: there is only the stderr stream at that point. Makes sense? Thank you, Ali
Oct 22 2020
prev sibling parent Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 20:33:45 UTC, Paul Backus wrote:

 Once an Error has been thrown, the program is in an invalid 
 state
No, not if it's an `AssertError` that occurs inside a `unittest` block [1] [1] https://dlang.org/spec/unittest.html -- /Jacob Carlborg
Oct 23 2020
prev sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 22 October 2020 at 20:23:49 UTC, Ali Çehreli wrote:
 1) Thread entry functions should catch and report any 
 Throwable. Otherwise, main thread will see only a timeout and 
 stack trace of the worker will lost.
Worth remembering catching actually loses valuable information when running in a debugger and/or with core dumps enabled. If you did want to do this, at a minimum it would have to honor the DRT-trapExceptions value.
Oct 22 2020
next sibling parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 10/22/20 1:38 PM, Adam D. Ruppe wrote:

 If you did want to do this, at a minimum it would have to honor the 
 DRT-trapExceptions value.
Great advice! Created a documentation issue for this nonexistent feature. :) https://issues.dlang.org/show_bug.cgi?id=21339 Ali
Oct 22 2020
parent Paolo Invernizzi <paolo.invernizzi gmail.com> writes:
On Thursday, 22 October 2020 at 20:58:50 UTC, Ali Çehreli wrote:
 On 10/22/20 1:38 PM, Adam D. Ruppe wrote:

 If you did want to do this, at a minimum it would have to 
 honor the DRT-trapExceptions value.
Great advice! Created a documentation issue for this nonexistent feature. :) https://issues.dlang.org/show_bug.cgi?id=21339 Ali
I definitely need to read 'This week in D' more constantly! :-P
Oct 23 2020
prev sibling next sibling parent reply Jacob Carlborg <doob me.com> writes:
On Thursday, 22 October 2020 at 20:38:32 UTC, Adam D. Ruppe wrote:

 Worth remembering catching actually loses valuable information 
 when running in a debugger and/or with core dumps enabled.
druntime should be able to detect if running inside a debugger.
 If you did want to do this, at a minimum it would have to honor 
 the DRT-trapExceptions value.
I guess this is the workaround to the above. -- /Jacob Carlborg
Oct 23 2020
parent Adam D. Ruppe <destructionator gmail.com> writes:
On Friday, 23 October 2020 at 09:24:14 UTC, Jacob Carlborg wrote:
 druntime should be able to detect if running inside a debugger.
I think that's actually impossible on Linux. But that's not even a great solution anyway, what if core dumps are enabled but it isn't in a debugger? There's a lot of scenarios that aren't clear cut.
Oct 23 2020
prev sibling parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 10/22/20 1:38 PM, Adam D. Ruppe wrote:

 On Thursday, 22 October 2020 at 20:23:49 UTC, Ali =C3=87ehreli wrote:
 1) Thread entry functions should catch and report any Throwable.
 Otherwise, main thread will see only a timeout and stack trace of the=
 worker will lost.
Worth remembering catching actually loses valuable information when running in a debugger and/or with core dumps enabled. If you did want to do this, at a minimum it would have to honor the DRT-trapExceptions value.
Once again, thank you. But I tried this and it seems to be effective=20 only on the main thread. So, in case of multiple threads, assuming that a thread threw Error, I=20 don't know any better solution than the following. The options are progressively more wishful. In theory, options beyond=20 'a' should not be attempted but option 'a' is not practical nor safe. a) Don't do anything. I think this choice is horrible because - The rest of the threads would still be continuing with presumably=20 invalid program state. - Nobody would know that a thread has died. - Nobody would know the stack trace when Error was thrown. b) Catch Error in the worker thread and call abort() immediately. Note:=20 In the strictest sense, even calling abort() should not be attempted but = this thinking is madness because otherwise the rest of the program would = be continuing with "invalid state." c) Same as 'b', but do stderr.writeln(err) before abort(). This way, we=20 *may* have an idea of what happened but in a safety-critical=20 application, we may be killing a human instead of printing anything. d) Catch Error and send it as a message to the main thread and let the=20 main thread do one of the above. Even though this feels the cleanest, I=20 don't see any benefit in doing this over the others. Over the years, I've been a part of many error management strategy=20 discussions on these forums and elsewhere I still don't know what to do. = For example, there are safety-critical programs out there that can't=20 even abort. The thinking is that instead of leaving a machine in its=20 "invalid state", which may kill humans, there is always a better way of=20 degrading the machine's operations to do something safer. For example,=20 in the case of autonomous driving, slowing down the vehicle slowly and=20 parking on the side of the road may be better than aborting at 120kph on = the highway. Always a fascinating discussion... Ali
Oct 23 2020
next sibling parent Adam D. Ruppe <destructionator gmail.com> writes:
On Friday, 23 October 2020 at 16:30:53 UTC, Ali Çehreli wrote:
 c) Same as 'b', but do stderr.writeln(err)
Note that stderr doesn't necessarily exist. A Win32 GUI application is likely to have stderr set to an invalid file descriptor even in the best conditions. but idk.
Oct 23 2020
prev sibling next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Fri, Oct 23, 2020 at 09:30:53AM -0700, Ali Çehreli via Digitalmars-d wrote:
[...]
 Over the years, I've been a part of many error management strategy
 discussions on these forums and elsewhere I still don't know what to
 do. For example, there are safety-critical programs out there that
 can't even abort.  The thinking is that instead of leaving a machine
 in its "invalid state", which may kill humans, there is always a
 better way of degrading the machine's operations to do something
 safer. For example, in the case of autonomous driving, slowing down
 the vehicle slowly and parking on the side of the road may be better
 than aborting at 120kph on the highway.
I think Walter's go-to answer on that one is, have a monitor process that detects when the main process has crashed, and immediately take over to initiate a safe shutdown procedure. Basically, the main process has entered an invalid state, so we can no longer assume it's able to correctly carry out the safe shutdown procedure (what if the bug that triggered the assert has also corrupted the program's data so that when it tries to slow down it accelerates instead?). So we use a redundant, independent component (the monitor process) that's known to be in a consistent state to do the safe shutdown. I'm not sure how to apply this principle (or if it's even applicable) to all situations, but there you have it. T -- Маленькие детки - маленькие бедки.
Oct 23 2020
parent =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 10/23/20 10:04 AM, H. S. Teoh wrote:

 I think Walter's go-to answer on that one is, have a monitor process
 that detects when the main process has crashed, and immediately take
 over to initiate a safe shutdown procedure.
And "immediately" must be a very short time.
 Basically, the main process
 has entered an invalid state, so we can no longer assume it's able to
 correctly carry out the safe shutdown procedure (what if the bug that
 triggered the assert has also corrupted the program's data so that when
 it tries to slow down it accelerates instead?). So we use a redundant,
 independent component (the monitor process) that's known to be in a
 consistent state to do the safe shutdown.
Yes, that's what's being done in the industry. But how to debug what happened? Perhaps leave a trace that explains what was being attempted, instead of what went wrong? I guess... Or perhaps, if possible, decouple the faulty program and let it attempt to give more information in a sand box.
 I'm not sure how to apply this principle (or if it's even applicable) to
 all situations, but there you have it.
Yeah, this kind of discussion better be attached to how safety-critical the system is. Yes, in theory even a format() will not work but it's ingrained in our assert() expressions and we use it all over the place and it's very practical: assert(c, format!"Can this work? %s"(i)); In practice it works and is extremely useful. Ali
Oct 23 2020
prev sibling parent Kagamin <spam here.lot> writes:
On Friday, 23 October 2020 at 16:30:53 UTC, Ali Çehreli wrote:
 c) Same as 'b', but do stderr.writeln(err) before abort().
This is most appropriate, basically the same as what druntime does.
Oct 23 2020
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
A related issue is:

https://github.com/dlang/dmd/pull/6896

where `scope` should catch Exceptions, not Throwables.
Oct 23 2020