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 python 3.8 #2418

Merged
merged 17 commits into from
Apr 19, 2024
Merged

Add python 3.8 #2418

merged 17 commits into from
Apr 19, 2024

Conversation

predragnikolic
Copy link
Member

related #2417
PLEASE DO NOT MERGE.

@predragnikolic
Copy link
Member Author

I am not sure how to address the CI error:

Traceback (most recent call last):
  File "./python3.8/unittest/loader.py", line 436, in _find_test_path
  File "./python3.8/unittest/loader.py", line 377, in _get_module_from_name
  File "/Users/runner/Library/Application Support/Sublime Text/Packages/LSP/tests/test_message_request_handler.py", line 2, in <module>
    from test_mocks import MockSession
  File "/Users/runner/Library/Application Support/Sublime Text/Packages/LSP/tests/test_mocks.py", line 5, in <module>
    from LSP.plugin.core.types import ClientConfig
  File "/Users/runner/Library/Application Support/Sublime Text/Packages/LSP/plugin/core/types.py", line 9, in <module>
    from wcmatch.glob import BRACE
ModuleNotFoundError: No module named 'wcmatch.glob'

@rchl
Copy link
Member

rchl commented Feb 13, 2024

Unittesting runs on PC3 I guess

@rwols
Copy link
Member

rwols commented Feb 13, 2024

wcmatch is a dependency that’s only available for py33.

@predragnikolic
Copy link
Member Author

wcmatch is a dependency that’s only available for py33.

It supports 3.8 ->
https://github.com/packagecontrol/channel/blob/8142a3b67d2ee640d8769cbfcec593e6e4df5554/repository.json#L1371

@rchl
Copy link
Member

rchl commented Feb 13, 2024

And as far as I understand only PC4 can understand that new schema.

rchl and others added 2 commits February 28, 2024 11:03
* main:
  Cut 1.29.0
  Fix usage of sublime.score_selector (#2427)
  docs: add info about typst-lsp commands (#2424)
@predragnikolic predragnikolic force-pushed the feat/py38 branch 3 times, most recently from e26dd90 to 20f8fc8 Compare March 21, 2024 18:12
@predragnikolic

This comment was marked as outdated.

@predragnikolic

This comment was marked as outdated.

@rwols
Copy link
Member

rwols commented Mar 21, 2024

Not minding the spam at all. In fact you’re doing some great work 👍

@predragnikolic

This comment was marked as outdated.

@deathaxe
Copy link
Contributor

deathaxe commented Mar 23, 2024

Changing the setting in view.settings().set("lsp_uri", "file:///foo/bar.txt") to something else (e.g.: view.settings().set("lsp_uri_2", "file:///foo/bar.txt") causes 1 or 2 tests to fail, but none gets stuck.

The problem seems to be related with the settings change listeners used by WindowConfigManager, which seem to lock up something.

I've recently saw comparable issues with conditions timing out on UnitTestings test cases itself every here and then on all OSs. They seem to have been related with test packages being created and removed for each test method individually. Creating and cleaning up all packages for each TestCase at once, fixed the issue. see: https://github.com/SublimeText/UnitTesting/pull/244/files

UnitTesting uses sublime.set_timeout() to schedule tests. Everytime a test method yields, another set_timeout() call is made to enqueue further steps, while giving core time to handle events.

Maybe we hit some limits of how many events/messages can be handled.

@deathaxe
Copy link
Contributor

deathaxe commented Mar 23, 2024

Seems to be a missing view instance, causing it. Using ViewTestCase fixes the issue.

see: https://github.com/deathaxe/LSP/actions/runs/8403796385/job/23014682385

https://github.com/deathaxe/LSP/blob/adf5f79a28fea3ede3bceb7cf5146a08d2b57c87/tests/test_configurations.py#L29

deathaxe and others added 3 commits March 24, 2024 23:00
This commit adopts `unittesting.ViewTestCase` to run view related test cases.

ViewTestCase ...

1. provides `view` and `window` members pointing to a dedicated view, being
   created for testing.
2. ensures not to close empty windows, which was probably the most likely
   reason for unittests failing before on MacOS.
@predragnikolic
Copy link
Member Author

Just to document.

CI was fixed with #2436 (comment)
by using a ViewTestCase:

ViewTestCase ...

  • provides view and window members pointing to a dedicated view, being created for testing.
  • ensures not to close empty windows, which was probably the most likely reason for unittests failing before on MacOS.

@predragnikolic
Copy link
Member Author

I also plan to add a release message.

@rwols
Copy link
Member

rwols commented Mar 25, 2024

Now it’s a safe next step to announce some kind of deadline date to do the transition to py38 for all helper packages with this ready to merge :)

Sublime Text once it finishes updating all packages. ⚠️

# Breaking changes
- We are transitioning LSP and LSP-* packages from Python 3.3 to Python 3.8.
Copy link
Member

Choose a reason for hiding this comment

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

Would include info on how this affects users.

I guess the most apparent side effect is that ST will require an extra restart for things to start working properly. Maybe the original warning should be updated to reflect that since for this case it will behave differently than normally.

Copy link
Member Author

Choose a reason for hiding this comment

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

How about this?

⚠️ After updating the LSP package, Sublime Text will need to be restarted more than once for things to work properly. ⚠️

# Breaking changes
- We are transitioning LSP and LSP-* packages from Python 3.3 to Python 3.8.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did expand the release message to include info about what the user can expect in this update
and what the user can do if problem arise during the update.

Copy link
Member Author

Choose a reason for hiding this comment

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

@predragnikolic predragnikolic merged commit 1ea635c into main Apr 19, 2024
8 checks passed
@predragnikolic predragnikolic deleted the feat/py38 branch April 19, 2024 16:14
@predragnikolic predragnikolic restored the feat/py38 branch April 19, 2024 17:30
@predragnikolic predragnikolic deleted the feat/py38 branch April 19, 2024 18:39
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.

4 participants