Skip to content

Add Windows CI + fix a bunch of bugs/tests on Windows - #1657

Open
progval wants to merge 29 commits into
masterfrom
ci-windows
Open

Add Windows CI + fix a bunch of bugs/tests on Windows#1657
progval wants to merge 29 commits into
masterfrom
ci-windows

Conversation

@progval

@progval progval commented Mar 8, 2026

Copy link
Copy Markdown
Owner

No description provided.

@progval progval changed the title Add Windows CI Add Windows CI + fix a bunch of bugs/tests on Windows Mar 8, 2026
@progval
progval force-pushed the ci-windows branch 2 times, most recently from f8dd6db to 1355f84 Compare March 8, 2026 11:56
not closing it prevent limnoria-test from cleaning the data dir on Windows
eg. this seems to be needed for Factoids' tests to close their DB.
Otherwise, ChannelDBHandler.die() is never called because IrcCallback.die()
does not call super().die() (and it can't because its parent class does not
have such a method)
They are noisy on Windows every time we spawn a process (and will be on POSIX when we'll switch to the 'forkserver' method).

It also seems pretty bad to run them in forked processes (with the 'fork' method), I imagine it might corrupt non-atomic files because it may cause concurrent writes; in addition to writing outdated data (if data in the main process was changed since the fork started)
This fixes tests for Aka and Network on Windows
codecs.open() is deprecated, and db.DBI.remove() seems to have issues with it, as it prepends '------' to the id instead of overwriting the id.
Comment thread plugins/GPG/plugin.py Outdated
Comment thread .github/workflows/test-windows.yml Outdated
Comment thread src/utils/file.py
def sanitizeName(filename):
"""Removes / from filenames and escapes them if they are '.' or '..'."""
filename = filename.replace('/', '')
r"""Removes '/', '\', and '|' from filenames and escapes them if they are '.' or '..'."""

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, should we do the same for all reserved chars on Windows <>:"/\|?*?

This feels quite restrictive though, so maybe there should be a config toggle which only takes effect outside of Windows.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need the same rules for people who want to switch between Linux and Windows

Comment thread .github/workflows/test-windows.yml Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants