Skip to content
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

Add try block to import readline #21

Merged
merged 2 commits into from Feb 26, 2024

Conversation

stuart-thomas-zoopla
Copy link
Contributor

Add a try block to import readline to improve windows compatibility.

Testing
Executing a Hello World .spl file in windows 11.

Before fix
image

After fix
image

Add a try block to import readline to improve windows compatibility
Run black an resolve formatting
@zmbc
Copy link
Owner

zmbc commented Feb 17, 2024

Thanks so much! Would you be willing to run all the tests locally on Windows, to make sure there isn't anything else broken?

@stuart-thomas-zoopla
Copy link
Contributor Author

image

All 30 failures are related to pexpect (full output attached testoutput.txt ). I've had a look and it seems to be another windows incompatibility and although there are a couple of windows ports of pexpect, neither seems to be up to date or working, and although pexpect has some windows compatibility, it doesnt support spawn. Can see more here https://pexpect.readthedocs.io/en/stable/overview.html#windows

I also had to downgrade the version of pyyaml being used to 5.3 in order to be able to get to a point where I could run the tests in windows due to an issue with compatibility with PEP517 builds. It looks like there are likely other packages (at least regex) that would also need investigating to see if there are compatibile versions, but doesnt seem worth it unless a fix for the pexpect issue can be found.

@zmbc
Copy link
Owner

zmbc commented Feb 26, 2024

Ah I see. pexpect is only used in the test suite -- if you don't mind, can you check whether you can run all the test programs directly (without error or obvious incorrect behavior)?

@zmbc zmbc merged commit 9495ffb into zmbc:main Feb 26, 2024
6 checks passed
@zmbc zmbc mentioned this pull request Mar 2, 2024
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.

None yet

2 participants