Closed RadWolfie closed 3 years ago
You have forgotten to add a space between &=
and assert_NTSTATUS
in tests_passed &=assert_NTSTATUS
.
NOTICE: Cxbx-Reloaded will crash as of commit cc1edc6. Except it does not crash on the hardware. The result can be found in op post.
NOTICE: Cxbx-Reloaded will crash as of commit cc1edc6.
In that case then I don't think we should merge this until the crash is solved. This, because the test suite is specifically used to test the kernel implementation of cxbxr. If it crashes, then we can't use it, which effectively makes it useless.
Or, see it as an incentive to fix the Cxbx-Reloaded kernel implementation
I used in the past the test suite to test the correctness of the implementation of RtlInitUnicodeString
in cxbxr after I updated it in https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/pull/1969. If this is merged as is, then I won't be able to do that again in the future. So, if you want this anyway, I instead propose to disable the test that causes the crash, with a comment which explains the reason for this, in order to still allow people to check the correctness of the cxbxr kernel implementation when it's updated, Once the crash is fixed, the test can be enabled again here.
@ergo720 I had been considering to fix the crash in currently active pull request https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/pull/2178. 😉 It's being work on at the moment.
I saw that this commit https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/pull/2178/commits/45e4e18415e5383dad4ce43d7c92503970aa9e46 supposedly fixes the crash that was previously reported. If confirmed, then we can merge this now.
EDIT; I got confirmation that the crash in cxbxr is fixed, so merging this.
Draft pull request due to missing status defines in nxdk's repo.
Add tests for IoCreateSymbolicLink and IoDeleteSymbolicLink against hardware to return expected result. From hardware test: (updated)
Cxbx-Reloaded does not respect expected return result, so it needs some work to make corrections.