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

🧪 initial tests #28

Merged
merged 5 commits into from
Dec 6, 2023
Merged

🧪 initial tests #28

merged 5 commits into from
Dec 6, 2023

Conversation

juftin
Copy link
Owner

@juftin juftin commented Nov 29, 2023

Summary

This PR adds some initial tests to the hatch-pip-compile plugin. The tests are a start and coverage isn't complete yet - more to come later.

Test fixtures were adapted from hatch's fixtures: https://github.com/pypa/hatch/blob/master/tests/conftest.py

Relates to #18

Base automatically changed from feat/install-upstream-env to main November 30, 2023 04:42
@juftin juftin force-pushed the tests/init branch 3 times, most recently from 54d662e to 430b2b8 Compare November 30, 2023 05:16
@juftin juftin marked this pull request as ready for review November 30, 2023 05:24
@juftin juftin force-pushed the tests/init branch 3 times, most recently from c6bd754 to 45655c0 Compare December 6, 2023 16:43
Comment on lines +344 to +354
def plugin_check_command(
self, command: Union[str, List[str]], *, shell: bool = False, **kwargs: Any
) -> CompletedProcess:
"""
Run a command from the virtualenv
"""
return self.virtual_env.platform.check_command(
command=command,
shell=shell,
**kwargs,
)
Copy link
Owner Author

@juftin juftin Dec 6, 2023

Choose a reason for hiding this comment

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

Separating this into its own function made this manageable to patch in testing.

Copy link
Owner Author

Choose a reason for hiding this comment

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

FYI @oprypin I'm going to merge this one through to start focusing on smaller tests to add

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure just do it :D

@juftin juftin force-pushed the tests/init branch 2 times, most recently from fe96f84 to 48173af Compare December 6, 2023 17:27
Comment on lines +23 to +33
expected_call = [
"python",
"-u",
"-m",
"pip",
"install",
"--disable-pip-version-check",
"--no-python-version-warning",
"-q",
"--requirement",
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider https://github.com/15r10nk/inline-snapshot/ to make future updates of expected values easier :)
Disclaimer: I actually haven't tried it out myself

@juftin juftin merged commit caeb28b into main Dec 6, 2023
6 checks passed
@juftin juftin deleted the tests/init branch December 6, 2023 18:11
@juftin
Copy link
Owner Author

juftin commented Dec 6, 2023

🎉 This PR is included in version 1.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@juftin juftin added the released label Dec 6, 2023
@oprypin
Copy link
Contributor

oprypin commented Dec 6, 2023

Mm I find it quite unreasonable that this pull request constitutes a release
https://github.com/juftin/hatch-pip-compile/releases/tag/v1.8.0
and the release notes basically should be empty because to users this is just noise

@juftin
Copy link
Owner Author

juftin commented Dec 6, 2023

I actually debated on whether or not to trigger a release for this, ultimately since e39e82d changed the code-path and needed a version increment. Right now my release mechanism only has patch releases for bug commits - and this wasn't a bug. Do you think this should've been a non-release change?

Good to know that you felt the release notes included noise - that can be cut back.

@oprypin
Copy link
Contributor

oprypin commented Dec 6, 2023

I see the point about the code path.. But I think that actually doesn't make sense. Just declare that this codebase is not meant to be used as an import in Python and that's that 😃

@oprypin
Copy link
Contributor

oprypin commented Dec 6, 2023

Or maybe I misunderstood what you meant..

semantic-release seems to define ✨ as something greater than a bugfix whereas this I think was something lesser than a bugfix because there's 0 impact for users

@juftin
Copy link
Owner Author

juftin commented Dec 6, 2023

That commit was probably something like 🎨 (code tidying) which didn't warrant a release - but some calls to self.platform got changed to self.virtual_env.platform so I ended making the call to push a release.

I get your point though - that shouldn't happen again.

@oprypin
Copy link
Contributor

oprypin commented Dec 6, 2023

Hmm well if it was a change in behavior then I guess it should be described, but the description is "plugin command runner". I actually didn't understand that this is a change.

Anyway just thoughts out loud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants