-
Notifications
You must be signed in to change notification settings - Fork 8
feat: ros backend tests #75
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?
feat: ros backend tests #75
Conversation
2510560
to
28ef57b
Compare
b07681f
to
e504199
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.
The ROS specificness of this PR looks good to me! @Hofer-Julian could you validate that the testing logic and repo changes are proper?
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.
Valentin and I live-reviewed this. Valentin will review the code himself again, and then I'll have another look
scripts/download-artifacts.py
Outdated
repodata_files = sorted( | ||
list(temp_dir.rglob("repodata.json")) | ||
+ list(temp_dir.rglob("repodata.json.zst")) | ||
) | ||
|
||
console.print(f"[blue]Found {len(backend_executables)} backend executable(s)") | ||
|
||
# Extract all executables | ||
for executable in backend_executables: | ||
final_path = output_dir / Path(executable).name | ||
if final_path.exists(): | ||
if final_path.is_dir(): | ||
shutil.rmtree(final_path) | ||
else: | ||
final_path.unlink() | ||
if not repodata_files: | ||
raise FileNotFoundError( | ||
"Could not locate a channel directory inside the artifact. " | ||
f"Archive contents: {file_list}" | ||
) | ||
|
||
zip_ref.extract(executable, output_dir) | ||
extracted_path = output_dir / executable | ||
channel_dir = repodata_files[0].parent.parent |
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 don't understand this 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.
Please adapt the README
dd776fd
to
34e1675
Compare
34e1675
to
55d8243
Compare
11be9fb
to
559bcb6
Compare
6c8aa6d
to
b548c83
Compare
This reverts commit 234f61f.
bb98e05
to
1544854
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.
We are close!!!!!!!
if channel_source.exists(): | ||
print("🧹 Removing existing channel directory before rebuilding") | ||
shutil.rmtree(channel_source) |
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.
Not needed anymore
is_windows = sys.platform.startswith("win") | ||
|
||
# Find the pixi binary | ||
some_repodata_file = None |
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 rename this to is_channel
env_override = os.environ.get("PIXI_TESTSUITE_BACKEND_CHANNEL") | ||
if env_override: | ||
return env_override |
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 remove this env var everywhere
if channel_dir.is_dir() and any(channel_dir.rglob("repodata.json")): | ||
return channel_dir.as_uri() | ||
|
||
return None |
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.
Raise exception instead
if local_uri is None: | ||
return copied_str |
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.
if local_uri is None: | |
return copied_str |
try: | ||
data = tomllib.loads(updated) | ||
except Exception: | ||
pass |
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.
Don't catch exception
return None | ||
|
||
|
||
def copy_manifest( |
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.
Please add doc comments
src: str | os.PathLike[str], | ||
dst: str | os.PathLike[str], | ||
*, | ||
follow_symlinks: bool = True, |
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 try to remove this
return copied_str | ||
|
||
|
||
def copytree_with_local_backend( |
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 would be nice to also do this within this function:
# Remove existing .pixi folders
shutil.rmtree(cmake_env_test_project.joinpath(".pixi"), ignore_errors=True)
Afterwards we can remove this rmtree call everywhere
def simple_workspace( | ||
tmp_pixi_workspace: Path, | ||
request: pytest.FixtureRequest, | ||
local_backend_channel_uri: str | None, |
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.
local_backend_channel_uri: str | None, | |
local_backend_channel_uri: str, |
Add tests on top of https://github.com/ruben-arts/ros_workspace/ that packages are being built, extra-input-globs are stored in
metadata.json
file and that file change triggers rebuild.Cross-dependent on prefix-dev/pixi-build-backends#395.