-
Notifications
You must be signed in to change notification settings - Fork 2
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
Integration tests for tools #43
base: merge-2023-04-02
Are you sure you want to change the base?
Conversation
9c7318d
to
5c3bab9
Compare
Codecov Report
@@ Coverage Diff @@
## main #43 +/- ##
===========================================
+ Coverage 60.97% 91.56% +30.59%
===========================================
Files 13 13
Lines 574 593 +19
===========================================
+ Hits 350 543 +193
+ Misses 224 50 -174
Continue to review full report at Codecov.
|
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.
We should be careful inside integration test, e.g. for that git config --global user.name
, IMO we should mock those values before running the tests.
Please, don't store the truth inside of targz archives, it makes it impossible to read/diff on github
from incipyt.tools.setuptools import LINUX_MIN_PYTHON_VERSION | ||
|
||
|
||
def make_archive(): |
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.
missing docstring, this test-file will be our integration-test showcase/template, it needs to be documented so people feel easy using it as a template for their own tests
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.
and please note: I understand nothing at this function, what the fuck is an isolated_filesystem
? why the fuck you need to chdir and remove a targz file???
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.
where this function is used?
shutil.unpack_archive( | ||
pathlib.Path(__file__).parent / "setuptools.tar.gz", "archive" | ||
) |
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.
I've just noticed that the "truth" is inside a tar.gz file tests/test_tools/setuptools.tar.gz. I think it is a bad idea, if one day we change something to the setuptools plugin, we'll have to adapt this tgz file but we won't have a clear diff of what changed.
Today's me want to read (inside github) what's inside that tgz file, future's me want to read a diff (inside github) of what changed inside that tgz file. Please don't store it as a tgz, just upload the whole folder as-is.
|
||
|
||
def diff_files(dircmp): | ||
result = {str(pathlib.Path(dircmp.left) / f) for f in dircmp.diff_files} |
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.
fspath
from incipyt import project | ||
|
||
|
||
def diff_files(dircmp): |
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.
what type is dircmp?
path | ||
for path in result | ||
if not any( | ||
re.match(pattern, path) for pattern in [r".*/\.env/.*", r".*/dist/.*"] |
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.
let's use or own .gitignore https://stackoverflow.com/a/22090594
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.
while I agree those need to be stored somewhere else (a list of globs, just as .gitignore does, is just fine), using incipyt own .gitignore seems ankward and error-prone to me
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.
also, I'm not sure a 3rd party lib is really needed when we already have glob in the stdlib
incipyt/tools/git.py
Outdated
self._set_git_credential_email = False | ||
self._set_git_credential_name = False |
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.
git doesn't use credentials, github does. user.name and user.email are not credentials
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.
a name that starts with a verb should be a function, not a boolean
incipyt/tools/git.py
Outdated
|
||
def post(self, workon): | ||
"""Run `git add --all`. | ||
|
||
:param workon: Work-on folder. | ||
:type workon: :class:`pathlib.Path` | ||
""" | ||
if self._set_git_credential_email: |
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.
ok now I understand.
So the commit message should be:
feat(git): user.name defaults to AUTHOR_NAME
Make sure there is no global git user.name and user.email: make sure
`git config --global user.name` is empty, same of `user.email`. Create a
project using git and setuptools: the wizard prompts you for an author
name and email as it couldn't determine them via git. When the project
is created, the author name and email are set inside `setup.cfg` but the
local git configuration still lacks an user.name and a user.email.
In this commit when no git user name or email is set globally and that
one was provided to the wizard, it automatically update the config of
the new git repository to set the user.name and the user.email to the
AUTHOR_NAME and AUTHOR_EMAIL.
328f61f
to
397edbc
Compare
first integration tests failing on windows CI is due to non platform-agnostic path matching at testing.py:23 (diff is |
incipyt/tools/git.py
Outdated
|
||
def post(self, workon): | ||
"""Run `git add --all`. | ||
|
||
:param workon: Work-on folder. | ||
:type workon: :class:`pathlib.Path` | ||
""" | ||
git_user_email_return = commands.run( |
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.
I would isolate the "update git config" in a dedicated function
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.
also amend the docstring
incipyt/tools/git.py
Outdated
commands.run( | ||
[ | ||
"git", | ||
"-C", | ||
os.fspath(workon), | ||
"config", | ||
"user.email", | ||
project.environ["AUTHOR_EMAIL"], | ||
] | ||
) |
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.
I veto that.
Running $ git -C {} config user.email {}
shouldn't expand over 9 lines of code.
def git(args, workon, **kwargs):
args = ["git", "-C", os.fspath(workon), *args]
return commands.run(args, **kwargs)
git = partial(git, workon=workon)
This work include two new utilities, one to create a `StringTemplate` out of a file located inside of the new `/templates` directory, the other to copy a template file directly to the project.
A software license is a file stored at the root of a project which explain what it is allowed to do with the project source code and under what conditions. e.g. a project is qualified as "open-source" when it is published with a license that allows to distribute and modify the project source code. Until now the license file generated by incipyt was a simple copyright notice which made it explicit that the current work was the intellectual property of its owner. Under internationnal laws, this forbid anyone from modifying the work without the express authorization of the author. In this work, we added a way so that the user can choose an open-source license instead of the copyright notice. We added `--license <license>` to the CLI arguments and bundled common open-source licenses such as MIT and GPL. The previous behavior, to simply include a copyright notice, is still possible thanks to `--license copyright`.
See https://docs.pytest.org/en/latest/how-to/fixtures.html#yield-fixtures-recommended pytest recommends using a yield construction to setup and teardown environments for tests. Here we use it to clean project.environ and project.structure to provide reliable initial project in any case.
d2cd54a
to
12736ae
Compare
12736ae
to
90a2f4e
Compare
90a2f4e
to
387fbda
Compare
Integration tests for basic tools: setuptools, venv and git.
PR enabled to check all tests on the CI server, DO NOT MERGE yet.
Update versions for CI (GitHub Actions and pre-commit).