Refactor PE parser and rewrite WMIC .exe version extraction #79
Conversation
The PE version extraction was refactored. dorkbox Parser was moved to its own class behind an interface. The WMIC extractor was reworked to call wmic.exe directly without creating temporary files, that should avoid potential issues with AV software and be faster. Tests added that rely on local java.exe and cmd.exe.
|
Nice. I personally didn't care enough to fix this, because you just need to set a manual path once, but it would still be nice to have. Will check and test locally later. I see you are contributing to various open source wc3 projects, cool 👍 |
Codecov Report
@@ Coverage Diff @@
## master #79 +/- ##
============================================
- Coverage 40.96% 40.85% -0.12%
- Complexity 1722 1724 +2
============================================
Files 432 437 +5
Lines 21996 22066 +70
Branches 1821 1824 +3
============================================
+ Hits 9010 9014 +4
- Misses 12437 12502 +65
- Partials 549 550 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
|
||
| @Test() | ||
| public void extractExeVersionTest() throws VersionExtractionException, Exception { | ||
| // These tests need some static .exe files we don't carry around currently. |
There was a problem hiding this comment.
I think we should rather ship some .exe file in test resources, then it can also work on CircleCI
There was a problem hiding this comment.
However not the WMIC part?.. The CI runs on Linux (or whatever but not Windows). I'm gonna look to create a small program with PE Version info.
There was a problem hiding this comment.
Hi, any update on this? You can disable the wmic part of course.
There was a problem hiding this comment.
Yes still "in progress". At first I wanted to create a cross-compiled executable from Linux, so replicatable builds would be easy. But I don't see any way at all, nor any tools, to modify Windows-specific PE data from Linux. I just finished cleaning up my workspace today, I will follow-up soon and make a simple static binary for tests. Cheers
There was a problem hiding this comment.
At last I made a C# binary with the version info: https://github.com/Luashine/dotnet-pe-version
Completely removed the cmd.exe/java.exe tests and now only use the embedded file. The WMIC test is skipped on non-Windows.
EDIT/PS: Maybe I should've kept the CMD.exe file test, because dorkbox PE fetched the more than just the Version field and that required manual removal.
Added a static binary so that the dorkbox peParser can always be tested. Windows-specific WMIC test will be properly skipped if not running Windows.
|
src/test/java/wc3libs/bin/GameExeTest.java That test still fails for me (#62), I will fix it after this PR |
|
Nice 👍 |
The PE version extraction was refactored.
dorkbox Parser was moved to its own class behind an interface. The WMIC extractor was reworked to call wmic.exe directly without creating temporary files, that should avoid potential issues with AV software and be faster.
Tests added that rely on local java.exe and cmd.exe.Update: At last I made a C# binary with the version info: https://github.com/Luashine/dotnet-pe-version
Completely removed the cmd.exe/java.exe tests and now only use the embedded file. The WMIC test is skipped on non-Windows.
It's been a while since I last touched Java, I'd love to hear feedback and a review.
I wanted to tackle #62 so this is part 1 basically. Probably overengineered the IO part of WMIC (just a tiny bit :P). When I tried to reenable GameExeTest, it throws an exception on my system. I will continue in #62