-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removing test24 #10
Comments
+1 |
Hej, if I remember the idea of this test is to cover the elf-file loader. removing it would leave this part of the simulator untested. best, |
I have looked into the elf loader and clearly we do have a distinction between binary executable and elf executables. However, all other tests only use the binary executable mode. Shouldn't we run all tests in both binary and elf mode, to ensure both modes work correctly? |
The actual assembler from LLVM would be able to generate ELF-files out of the box, but then you might as well rely on the compiler. An other option would be to add the ability to emit ELF-files in the small assembler that is part of the simulator source code. I am not sure how complicated it would be to generate an ELF-binary using libelf. It should not be that hard. Btw., once the file is loaded there is no difference in terms of the simulator. So, there is no need to run the tests using both flat binary files and ELF-files. test24 is really only there to test the ELF-loader nothing else. |
I'll have a look at the assembler to see if it could easily be updated to generate elfs. I understand that test24 is just there to test the loading of elfs, but I think we should have more stringent testing of this. In theory, the elf loader should load the exact same memory as the binary loader given the same program. Our testing does not cover this. We basically only test that the elf loader loads that specific program correctly. |
TLDR: The test named
test24
is a special test that I think we should remove.test24
is comprised of the files:tests/test24.c
tests/test24.elf
tests/test24.in
What it does is repeat the first 8 characters sent to it through the UART and nothing else. However, the problem is that
tests/test24.elf
is committed in git. This is the result of compilingtests/test24.c
by hand withpatmos-clang
.I think this has no place in the simulator, as the test is not automatically updated if you change the
.c
file.Additionally, it means you need to have the compiler installed if you want to modify it or make the compilation part of the testing . It would be a bad idea to need to have the compiler installed during testing as it then introduces a circular dependency between the compiler and simulator.
Lastly, this test doesn't even test anything special, as
test22
tests reading from UART andtest23
test outputting to UART.Unless someone has a reason why this test should stay, I will be removing it to simplify the testing setup.
The text was updated successfully, but these errors were encountered: