Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ ignore-words-list = "haa,slq,collapsable,buss,reacher,thirdparty"
markers = [
"isaacsim_ci: mark test to run in isaacsim ci",
"device_split: re-invoke this file once per device (CPU and GPU) in CI due to process-global device locks (e.g., ovphysx<=0.3.7 gap G5)",
"windows_ci: mark test to run on Windows platforms in CI",
"arm_ci: mark test to run on ARM platforms in CI (e.g. NVIDIA DGX Spark)",
Comment on lines +200 to +201

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this better than a specific shard?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not exactly sure what a shard means here. Does it mean selecting the tests at workflow level? If so, I am trying to make it obvious at test level all the configuration.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Here we'd mark specific tests in our CI for windows / arm right? What I'm wondering is if we setup a specific job for windows that would list the test files we want to run. It seems weird to me to only mark a couple of tests here and here across the code base for these platform tests. Or it's not how this works and these markers are used to mark files instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes both. It can opt in at file level or at per test level. mainly two problems

  1. avoid modification at workflow level to include tests, so it's verbose at per test/file level that it's included for cross platform runs, which might have more traction.
  2. the CI resources are skewed. The current setup has up to 10 windows machines and 2 spark. It might be better to auto-include new tests but being conservative for the initial setup.

]

# Add pypi.nvidia.com so that `uv pip install isaaclab[isaacsim]` works without --extra-index-url.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Skip changelog: CI/test-infrastructure foundation (no user-facing API change). Registers the windows_ci and arm_ci pytest markers in pyproject.toml, teaches AppLauncher to recognize them in argv so they do not leak into Isaac Sim's argparse, and moves the AssetConverterBase USD scratch dir from hardcoded /tmp/IsaacLab to tempfile.gettempdir() for cross-platform compatibility. Test tagging and the cross-platform workflow files (arm-ci.yaml, windows-ci.yaml) ship in follow-up PRs.
10 changes: 8 additions & 2 deletions source/isaaclab/isaaclab/app/app_launcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -1193,12 +1193,18 @@ def _create_app(self):
sys.stdout = open(os.devnull, "w") # noqa: SIM115

# pytest may have left some things in sys.argv, this will check for some of those
# do a mark and sweep to remove any -m pytest and -m isaacsim_ci and -c **/pyproject.toml
# do a mark and sweep to remove any -m pytest, -m isaacsim_ci, -m windows_ci, -m arm_ci,
# and -c **/pyproject.toml
indexes_to_remove = []
for idx, arg in enumerate(sys.argv[:-1]):
if arg == "-m":
value_for_dash_m = sys.argv[idx + 1]
if "pytest" in value_for_dash_m or "isaacsim_ci" in value_for_dash_m:
if (
"pytest" in value_for_dash_m
or "isaacsim_ci" in value_for_dash_m
or "windows_ci" in value_for_dash_m

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are these new annotators necessary for this check? I think this was initially added for the isaac sim pipeline to work properly by making sure the correct arguments gets passed over. will that also be impacted by the windows and arm annotations?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's for pytest selecting tests, which uses the same mechanism as isaacsim_ci tag. While this is not quite scalable at the moment, some investigation into argparse refactor should help. e.g. to only pass appLauncher recognized arguments to it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are we not running the the whole batch of tests on windows? Would it make sense to make a specific test shard just for windows? (that would remove the annotation)

or "arm_ci" in value_for_dash_m
):
Comment on lines +1202 to +1207

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 The bare windows and arm markers are registered in pyproject.toml but are not included in the argv-stripping filter. If a developer runs pytest -m windows (or -m arm) on a test that also initialises AppLauncher, those tokens will leak into Isaac Sim's argparse — the same failure mode this block was originally written to prevent.

Suggested change
if (
"pytest" in value_for_dash_m
or "isaacsim_ci" in value_for_dash_m
or "windows_ci" in value_for_dash_m
or "arm_ci" in value_for_dash_m
):
if (
"pytest" in value_for_dash_m
or "isaacsim_ci" in value_for_dash_m
or "windows_ci" in value_for_dash_m
or "windows" in value_for_dash_m
or "arm_ci" in value_for_dash_m
or "arm" in value_for_dash_m
):

indexes_to_remove.append(idx)
indexes_to_remove.append(idx + 1)
if arg.startswith("--config-file=") and "pyproject.toml" in arg:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import os
import pathlib
import random
import tempfile
from datetime import datetime

from isaaclab.sim.converters.asset_converter_base_cfg import AssetConverterBaseCfg
Expand All @@ -34,9 +35,10 @@ class AssetConverterBase(abc.ABC):
can be set to True.

When no output directory is defined, lazy conversion is deactivated and the generated USD file is
stored in folder ``/tmp/IsaacLab/usd_{date}_{time}_{random}``, where the parameters in braces are generated
at runtime. The random identifiers help avoid a race condition where two simultaneously triggered conversions
try to use the same directory for reading/writing the generated files.
stored in folder ``<tempdir>/IsaacLab/usd_{date}_{time}_{random}``, where ``<tempdir>`` is the system
temporary directory (e.g. ``/tmp`` on POSIX, ``%TEMP%`` on Windows) and the parameters in braces are
generated at runtime. The random identifiers help avoid a race condition where two simultaneously
triggered conversions try to use the same directory for reading/writing the generated files.

.. note::
Changes to the parameters :obj:`AssetConverterBaseCfg.asset_path`, :obj:`AssetConverterBaseCfg.usd_dir`, and
Expand Down Expand Up @@ -64,9 +66,9 @@ def __init__(self, cfg: AssetConverterBaseCfg):

# resolve USD directory name
if cfg.usd_dir is None:
# a folder in "/tmp/IsaacLab" by the name: usd_{date}_{time}_{random}
# a folder in the system temp dir by the name: IsaacLab/usd_{date}_{time}_{random}
time_tag = datetime.now().strftime("%Y%m%d_%H%M%S")
self._usd_dir = f"/tmp/IsaacLab/usd_{time_tag}_{random.randrange(10000)}"
self._usd_dir = os.path.join(tempfile.gettempdir(), "IsaacLab", f"usd_{time_tag}_{random.randrange(10000)}")
else:
self._usd_dir = cfg.usd_dir

Expand Down
Loading