-
Notifications
You must be signed in to change notification settings - Fork 783
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 proof of concept formatter tests #426
Conversation
pytest is much more mature and has many additional useful features. it
would also allow you to get rid of the run_test.py
otherwise excited for this to make it in to the repo!
…On Sat, May 8, 2021, 6:04 AM splondike ***@***.***> wrote:
This pull request implements a proof of concept for running unittests
outside of the Talon environment (e.g. via github actions) while making
sure we don't disturb things when running within Talon. You can see I've
got a single test case on one of the formatters running and passing.
Some things to consider:
1. Is the built in unittest module fine or do we want to use pytest?
If everybody thinks this looks like a good addition I'll add something to
the README and merge it in. The next step down this journey (aside from
writing more tests) would be setting up #381
<#381> and integrating
with that.
------------------------------
You can view, comment on, or merge this pull request online at:
#426
Commit Summary
- Implement proof of concept formatter tests
File Changes
- *A* .gitignore
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-bc37d034bad564583790a46f19d807abfe519c5671395fd494d8cce506c42947>
(3)
- *A* code/__init__.py
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-d2f5ec6f3ef8692646d0c77828185520646591fe1e687b3d00caa489e00ff1a6>
(1)
- *A* run_tests.py
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-5d20bf301d1d889d7ec73948673520e35c59a2f06b78ea646f0d7f0348ff6669>
(25)
- *A* tests/stubs/README
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-b3b89d9aa834353ac5f57061bce236935f5d869f14f37818abdc6c02522a83fb>
(3)
- *A* tests/stubs/talon/__init__.py
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-d60346882b4c5c1ce781e9f6b90d727aa7f5cf2a17c40056ff5462a95b11213c>
(100)
- *A* tests/stubs/talon/grammar.py
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-18016ea53d76fa1f1b8b2ee282ed9ca2141e7f67dffa7310163c6ffe04b6d204>
(3)
- *A* tests/test_formatters.py
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-f28b3ed23223b000771c34964f74ea35379b271cc8b4fe4191cd0c637b621fbd>
(23)
- *A* tests/utils.py
<https://github.com/knausj85/knausj_talon/pull/426/files#diff-9e08e8c7769db7b58089fd7659d75ca96df40bb7b5d50299d3a30abd54da9ad6>
(7)
Patch Links:
- https://github.com/knausj85/knausj_talon/pull/426.patch
- https://github.com/knausj85/knausj_talon/pull/426.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#426>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAHGZXG5A6MQWF62QYMBVDTMUEDBANCNFSM44MSUA3Q>
.
|
This looks like a great start.
|
OK, I'll give pytest a try on the weekend. Yeah, I don't have a compelling case for testing within the Talon runtime currently, I won't worry about it for the moment. I don't expect we'll have too many tests in this repo anyway. |
Okay so I have swapped it over to using pytest. You can run it using There was a bit of a complication setting it up because pytest uses the built in Python code module (unlike unittest). This means that by the time my test code is running and wants to import from the knausj code module the other one is already registered. I worked around this using a symlink. See the If this looks good to y'all now I'll add something to the README about how to run it and then merge it. |
tests/stubs/README
Outdated
@@ -0,0 +1,3 @@ | |||
This directory contains stubs of Talon APIs. It is included via run_tests.py so we can run our test cases outside the Talon environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Faulty comment.
conftest.py
Outdated
# look at customising the python module loader: | ||
# https://docs.python.org/3/reference/import.html | ||
# I've had a go at relative imports without success. | ||
symlink_src = os.path.join(curr_dir, "code") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From aegis:
just need a PathFinder in sys.meta_path I think
New
8:39
looks something like this:
class PluginFinder(PathFinder):
@classmethod
def find_spec(cls, fullname, path=None, target=None):
return super().find_spec(fullname, path, target)
sys.meta_path.insert(0, PluginFinder)
(edited)
8:40
if you're passed a None path, you need to use the fullname to figure out the file path. otherwise, check if the path makes sense for a path you are willing to load from (edited)
8:40
and find_spec can return None if you couldn't find anything
perhaps:
- from knausj_code import formatters
+ from user.knausj_talon.code import formatters
8:42
so then you take any fullname starting with user.knausj_talon. and rebase it to the root of your repo
you'd probably also handle from talon import ... from the PluginFinder
@lunixbochs Re-implemented the pytest import manipulations as an importlib loader. It's a little bit clunky with the root module loader, but seems to work OK. Ready for re-review. |
should the draft window tests use pytest too? |
Yeah, probably should though pytest happily runs the unittest format as well. Sounds like nobody has objections to the shape of this PR so I'll do that README modification and merge it in on the weekend. Will probably swap the draft window tests to pytest also then. After that I'm going to look into the formatters a bit more, I'm not sure they do the right thing with symbols. And after that maybe Github Actions so we can have a CI. |
This pull request implements a proof of concept for running unittests outside of the Talon environment (e.g. via github actions) while making sure we don't disturb things when running within Talon. You can see I've got a single test case on one of the formatters running and passing.
Some things to consider:
If everybody thinks this looks like a good addition I'll add something to the README and merge it in. The next step down this journey (aside from writing more tests) would be setting up #381 and integrating with that.