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

Fix Windows CI #147

Merged
merged 16 commits into from Aug 11, 2020
Merged

Fix Windows CI #147

merged 16 commits into from Aug 11, 2020

Conversation

foster999
Copy link
Contributor

@foster999 foster999 commented Aug 8, 2020

  • Non-posix paths are causing CI to fail on Windows. Converted to posix

  • sphinx_abs_dir tries to get the relative path between two Windows drives when testing. Changed to use absolute path on Windows

  • tests check that sphinx_abs_dir output is relative path using assert_node. Updated test_download_role to check if Windows absolute path is equivalent to the relative reftarget.

Fixes #143, fixes #54
Should fix some issues in Windows tests at executablebooks/jupyter-book#723

@foster999 foster999 marked this pull request as ready for review August 8, 2020 11:31
Copy link
Member

@akhmerov akhmerov left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

Would you like to similarly convert everything to Path methods? That would also close #54

jupyter_sphinx/ast.py Outdated Show resolved Hide resolved
jupyter_sphinx/execute.py Outdated Show resolved Hide resolved
@akhmerov
Copy link
Member

akhmerov commented Aug 9, 2020

I see the failures in Python 3.5. It's at its EoL in September, so I think we should drop it.

@foster999
Copy link
Contributor Author

I see the failures in Python 3.5. It's at its EoL in September, so I think we should drop it.

I think they could be genuine fails on Linux paths - I'd only tested locally on Windows py3.8. Will look into it

@foster999
Copy link
Contributor Author

Fixed a couple of other bugs I'd created, but this passes on Ubuntu py3.6. @akhmerov are we able to trigger the other CI checks to double check this, before we drop 3.5?

@akhmerov
Copy link
Member

akhmerov commented Aug 9, 2020

It's annoying since the jobs get canceled as soon as one fails. Yet, 3.7 and 3.8 work, and I think so should 3.6.

@akhmerov
Copy link
Member

akhmerov commented Aug 9, 2020

So I'd say let's go and drop 3.5.

Copy link
Member

@akhmerov akhmerov left a comment

Choose a reason for hiding this comment

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

I have some final suggestions and questions, other than that everything looks good, thanks!

jupyter_sphinx/ast.py Outdated Show resolved Hide resolved
jupyter_sphinx/thebelab.py Outdated Show resolved Hide resolved
jupyter_sphinx/utils.py Show resolved Hide resolved
tests/test_execute.py Outdated Show resolved Hide resolved
foster999 and others added 4 commits August 11, 2020 20:08
Co-authored-by: Anton Akhmerov <anton.akhmerov@gmail.com>
Co-authored-by: Anton Akhmerov <anton.akhmerov@gmail.com>
Copy link
Member

@akhmerov akhmerov left a comment

Choose a reason for hiding this comment

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

Great job, thanks a lot! Merging.

@akhmerov akhmerov merged commit 6eb309d into jupyter:master Aug 11, 2020
@foster999 foster999 deleted the fix/143/windows-ci branch August 11, 2020 20:23
@akhmerov akhmerov mentioned this pull request Sep 21, 2020
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.

Potential issue with sphinx_abs_dir on Windows maint: use pathlib
2 participants