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

test: rename tests to use identifiers #2044

Merged
merged 3 commits into from
Dec 30, 2022
Merged

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Dec 30, 2022

This PR fixes a minor annoyance that both @lobis and I have run in to. PyCharm, our preferred editor, requires that tests are importable in order to correctly detect the PyTest configuration.

If no-one has a strong reason to keep our non-identifier naming convention, this makes it possible to run individual unit-tests from PyCharm's test runner
image

@codecov
Copy link

codecov bot commented Dec 30, 2022

Codecov Report

Merging #2044 (6dc1c4c) into main (492368e) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@agoose77 agoose77 temporarily deployed to docs-preview December 30, 2022 01:17 — with GitHub Actions Inactive
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

So this means no more hyphens in test file names? That's okay, and I'd be surprised if anyone else on the team has an opinion on that.

I started using hyphens because it allows for some readability distinctions in names: e.g. "test_9999-change-from_buffers-arguments.py" where "from_buffers" is an identifier in a sentence or phrase, with hyphens taking the role of spaces (and sometimes dots: "change-ak-from_buffers").

Emacs Lisp has trained me to see hyphens as spaces, with function names like tool-bar-add-item-from-menu and variable names like switch-to-buffer-preserve-window-point.

Hyphens in file names can also be a good way to ensure that a script is a script, not importable as a module. But you want to do the opposite here. So yes, let's merge this!

@agoose77
Copy link
Collaborator Author

In general, I like the fact that we have hyphenated names (for the reasons that you give). But, as you note, I want to change that in this case to solve a tooling problem :)

@agoose77 agoose77 merged commit 64377c1 into main Dec 30, 2022
@agoose77 agoose77 deleted the agoose77/test-use-identifier branch December 30, 2022 17:01
@agoose77 agoose77 mentioned this pull request Dec 30, 2022
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