-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
Implement makeWin98Image #9
Conversation
You should add a repeatability test, since it will prove it works 100 times like the rest, I'm pretty impressed though. |
Done. Do I need to run it myself or does the Hercules CI do it? |
@io12 I have to bring your PR into the repo as a branch, which I just did, now the CI will start running. https://hercules-ci.com/github/MatthewCroughan/NixThePlanet/jobs/351 |
It seems to not be repeatable unfortunately due to keypresses dropping. I'll try to fix it. |
@io12 Yeah, one of the todos is perhaps to try xdotool to get the keypresses to stop dropping, I've found vncdo to be terrible. |
4a93463
to
898901e
Compare
Okay, I think it's more reliable now. Can you pull from the branch to run the CI again? |
Looks like it has worked 21 times so far, waiting for the 100, and then we can merge and document it :) |
@io12 It appears that 10 of the instances have gotten stuck at the following step |
Windows/msdos releases in the repo that I've tested, only 1 in 1000 would fail due to what seems like runtime bugs with dosbox, which is fine. But the other failed states can be worked around. I'm happy with 1 in 1000 failure rate, maybe even 1 in 100 is good enough for the purposes of the repo. |
Kind of frustrating that it's stuck at the final step 🙃. I'm not sure whether it's not fully booted yet and isn't ready to open the start menu or some other issue. Also, how did you get the screenshots? |
Nix does its work in /tmp, so I just go in there and extract the screenshots, same way as the macos repeatability test defined in this repo works. |
I made more repeatability improvements. Can you run the CI again? |
@io12 some fail like this, maybe dosbox's fault? Some seem to still get stuck at the end like this |
@io12 Statistically, your change did have an impact, as now at least 68 in 100 attempts work, as opposed to the previous 22. |
Maybe we could just |
That would work but then there's a ScanDisk improper shutdown message on the next boot. I think the issue is sometimes when the start menu is open before the "welcome to windows" window, the start menu can close. I think maybe the failing cases try to select the shutdown option after this happens. I'm trying to fix this by repeating the ctrl-esc u enter combination until the desktop icons and start menu disappear. |
@io12 rarely, this failure happens |
Okay, I think I fixed both issues and it repeats locally 16 times. Hopefully it works all 100 times now. |
Wow, the failure rate now seems to be 3 in 100. The three that have failed, have failed in a strange way though, as they have gotten to the
There are the last three screenshots that were taken by the expect script, but the screnshots are no longer being taken by the process, as their last modified time is no longer updating periodically. |
Thanks! Such a rare issue is pretty tricky to debug, but I removed a code path left over from a different approach that triggers in all three and doesn't trigger normally. Even if it doesn't fix the issue it should at least give more up-to-date screenshots. |
Also, I wonder how hard it would be to patch dosbox-x to fix the non-repeatability issues. If the issue comes from the timing of keypresses, maybe we can make dosbox-x take an input file that automatically triggers keypresses at specified cycle counts? I'm not sure if this would actually make things fully deterministic, but maybe worth a shot? And I don't think this is the first time Nix packages have patched build tools to make them more deterministic? |
@io12 Have you got a matrix or discord we can talk more actively on? |
1c5ebc7
to
fc1f2b5
Compare
I tested this only on x86_64-linux. Sorry if some things weren't done the best way.