intel
Missing test coverage for two functional changes
Describe the bug
Both #14273 and #14808 contain functional changes, but no tests to cover them which is a violation of our contribution guildelines
To reproduce
No response
Environment
No response
Additional context
No response
Hope you're doing well Alexey! Given that most of these changes are try-catches in the destructor, how I go about testing this?
Personally I am unsure: To properly cover all branches, I would have to somehow manually trigger exceptions in the destructor; I am personally not aware of a clean way to do this. I also thought about doing this as a part of a lit test checking for the IR generated by try-catch, but I don't really see how doing that would be productive, as it still doesn't check the logic is right.
Given that most of these changes are try-catches in the destructor, how I go about testing this?
Let's use device as an example. Can't you create such object within a {} scope so it is destructed when execution leaves that scope? Or just directly call its destructor?
Perhaps we can create device_impl object directly?
To properly cover all branches, I would have to somehow manually trigger exceptions in the destructor; I am personally not aware of a clean way to do this.
try {
// TODO catch an exception and put it to list of asynchronous exceptions
const PluginPtr &Plugin = getPlugin();
ur_result_t Err = Plugin->call_nocheck(urDeviceRelease, MDevice);
__SYCL_CHECK_OCL_CODE_NO_EXC(Err);
} catch (std::exception &e) {
__SYCL_REPORT_EXCEPTION_TO_STREAM("exception in ~device_impl", e);
}
Our unit-tests infrastructure allows to redefine behavior of UR adapters/plugins, so you can make a custom urDeviceRelease implementation which always returns an error which is then automatically converted into an exception.
I'm not entirely sure how to grab stderr/stdout from unit-tests to check that we have printed a message, but we should be able to at least check that abort wasn't called as part of default exception handling for uncaught exceptions within noexcept functions - simply by the fact that test finished with non-zero exit code.