You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In Java, it is possible to turn on and off asserts in a production build, they aren't simply compiled out of the build. They are turned on during testing. What this effectively means is that there are a whole suite of tests (namely anything that is using System.Diagnostics.Debug.Assert() currently) that we are completely skipping. To make matters even more complicated, some parts of the test framework are designed to catch the AssertionError that is thrown from those asserts when they fail and ignore them, and other parts are designed to fail the test in those cases.
I recently "fixed" a related issue (#299) by throwing InvalidOperationException, but I see that may have been the wrong approach, since the test framework has different behavior for AssertionException and InvalidOperationException in some cases.
I have been considering ways of reproducing the Java assertion behavior without producing negative performance impacts in production. But one of the main things to note is that Debug.Assert() is implemented as a regular function in .NET, meaning that both parameters are resolved first before it is called. Putting an expensive function call and/or expensive string building operation there is what is causing this problem in Debug builds. In Java, the asserts are not implemented as a function, and I suspect the compiler doesn't run the string building operation unless the assert fails, and I am sure neither of them are run if assertions are disabled.
What is needed is to come up with a solution that allows us to turn on asserts during testing in a way that doesn't hamper debug or runtime performance. One option I have been considering is to create a wrapper for Debug.Assert, something like:
internalstaticclassDebugging{publicstaticboolAssertsEnabled{get;set;}=SystemProperties.GetPropertyAsBoolean("assert",false);[MethodImpl(MethodImplOptions.AggressiveInlining)]publicstaticvoidAssert(Func<bool>conditionFactory,Func<string>messageFactory){if(AssertsEnabled){if(!conditionFactory())thrownewAssertionException(messageFactory());}else{Debug.Assert(conditionFactory(),messageFactory());// Note this line is completely removed from Release builds}}}
I suspect to get optimal production performance, we will probably also have to duplicate the AssertsEnabled check, even though it is not DRY. That will completely cut off the execution path to the fallback Debug.Assert() call in debug mode, but being that it is implemented as a function, it is probably best that we don't include it for debugging anyway and just rely on turning assertions "on" or "off".
Do note that the AssertionException already exists in the test framework. I have been trying to avoid putting testing code in the release, but it appears in order to duplicate this behavior we will either need to or come up with a solution that involves injecting a class for testing purposes or include it in the release code. Certainly to turn "on" and "off" asserts in production, it would be easier to follow the former approach.
In Java, it is possible to turn on and off asserts in a production build, they aren't simply compiled out of the build. They are turned on during testing. What this effectively means is that there are a whole suite of tests (namely anything that is using
System.Diagnostics.Debug.Assert()
currently) that we are completely skipping. To make matters even more complicated, some parts of the test framework are designed to catch theAssertionError
that is thrown from those asserts when they fail and ignore them, and other parts are designed to fail the test in those cases.I recently "fixed" a related issue (#299) by throwing
InvalidOperationException
, but I see that may have been the wrong approach, since the test framework has different behavior forAssertionException
andInvalidOperationException
in some cases.I have been considering ways of reproducing the Java assertion behavior without producing negative performance impacts in production. But one of the main things to note is that
Debug.Assert()
is implemented as a regular function in .NET, meaning that both parameters are resolved first before it is called. Putting an expensive function call and/or expensive string building operation there is what is causing this problem in Debug builds. In Java, the asserts are not implemented as a function, and I suspect the compiler doesn't run the string building operation unless the assert fails, and I am sure neither of them are run if assertions are disabled.What is needed is to come up with a solution that allows us to turn on asserts during testing in a way that doesn't hamper debug or runtime performance. One option I have been considering is to create a wrapper for
Debug.Assert
, something like:Which can be used like:
I suspect to get optimal production performance, we will probably also have to duplicate the
AssertsEnabled
check, even though it is not DRY. That will completely cut off the execution path to the fallbackDebug.Assert()
call in debug mode, but being that it is implemented as a function, it is probably best that we don't include it for debugging anyway and just rely on turning assertions "on" or "off".Do note that the
AssertionException
already exists in the test framework. I have been trying to avoid putting testing code in the release, but it appears in order to duplicate this behavior we will either need to or come up with a solution that involves injecting a class for testing purposes or include it in the release code. Certainly to turn "on" and "off" asserts in production, it would be easier to follow the former approach.Originally posted by @NightOwl888 in #308 (comment)
The text was updated successfully, but these errors were encountered: