-
Notifications
You must be signed in to change notification settings - Fork 6
feat: git source test #56
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
base: main
Are you sure you want to change the base?
Conversation
30ad6d1
to
64e3f95
Compare
Sorry @remimimimimi we already changed the way this works 😇 |
64e3f95
to
5b93fac
Compare
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.
thanks Valentin, LGTM!
can we remove the |
5cb5755
to
64c932e
Compare
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.
Looks good, I've added a few comments
version = "0.1.0" | ||
|
||
[package.build] | ||
source = { git = "https://github.com/prefix-dev/pixi-build-testsuite.git", rev = "ee87916a49d5e96d4f322f68c3650e8ff6b8866b", subdirectory = "tests/data/pixi_build/cpp-with-path-to-source/project" } |
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.
It should be possible to point to a git directory via a path.
That way we don't have to make a network request and the test will run faster
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, let's use the much simpler tests/data/pixi_build/minimal-backend-workspaces/pixi-build-cmake/
instead
channels = [ | ||
"https://prefix.dev/pixi-build-backends", | ||
"https://prefix.dev/conda-forge", | ||
] |
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.
The backend will be overriden anyway so no need to specify channels for it
channels = [ | |
"https://prefix.dev/pixi-build-backends", | |
"https://prefix.dev/conda-forge", | |
] |
@pytest.mark.slow | ||
def test_git_path(pixi: Path, build_data: Path, tmp_pixi_workspace: Path) -> None: | ||
""" | ||
Test git path in `[package.build.source]` | ||
""" | ||
project = "cpp-with-git-source" | ||
test_data = build_data.joinpath(project) | ||
|
||
shutil.copytree(test_data, tmp_pixi_workspace, dirs_exist_ok=True, copy_function=shutil.copy) | ||
|
||
verify_cli_command( | ||
[ | ||
pixi, | ||
"build", | ||
"-v", | ||
"--manifest-path", | ||
tmp_pixi_workspace, | ||
"--output-dir", | ||
tmp_pixi_workspace, | ||
], | ||
) | ||
|
||
# Ensure that exactly one conda package has been built | ||
built_packages = list(tmp_pixi_workspace.glob("*.conda")) | ||
assert len(built_packages) == 1 |
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 think it makes more sense to make the test similar to this:
pixi-build-testsuite/tests/integration_python/test_backends.py
Lines 20 to 35 in c37aa75
def test_pixi_minimal_backend(pixi_project: Path, pixi: Path, tmp_pixi_workspace: Path) -> None: | |
# Remove existing .pixi folders | |
shutil.rmtree(pixi_project.joinpath(".pixi"), ignore_errors=True) | |
# Copy to workspace | |
shutil.copytree(pixi_project, tmp_pixi_workspace, dirs_exist_ok=True) | |
# Get manifest | |
manifest = get_manifest(tmp_pixi_workspace) | |
# Install the environment | |
verify_cli_command( | |
[pixi, "run", "-v", "--locked", "--manifest-path", manifest, "start"], | |
stdout_contains="Build backend works", | |
) | |
That way you test that the package actually works
Tests for prefix-dev/pixi#4446.