-
Notifications
You must be signed in to change notification settings - Fork 106
Open
Description
Problem Description
The current architecture detection implementation in tests/containers/workbenches/jupyterlab/jupyterlab_test.py
is inefficient and lacks centralization:
Current Issues
- Performance Inefficiency: The
test_pdf_export
method starts and stops a container solely for architecture detection (uname -m
) on every test run - Code Duplication: Architecture detection logic will likely be duplicated across multiple test methods as more architecture-specific limitations are discovered
- No Central Configuration: Architecture-specific feature limitations (like PDF export on s390x) are scattered throughout the codebase without a single authoritative source
Impact Analysis
- Unnecessary container overhead on every test execution
- Potential for inconsistent architecture handling across different tests
- Maintenance burden when adding new architecture-specific limitations
- Reduced test suite performance due to redundant container operations
Solution Options
Option 1: Helper Function with Session Caching
Create a helper function with pytest session-level caching:
@pytest.fixture(scope="session")
def container_architecture(jupyterlab_image):
"""Cache architecture detection per test session."""
container = WorkbenchContainer(image=jupyterlab_image.name, user=4321, group_add=[0])
container.start(wait_for_readiness=False)
try:
exit_code, arch_output = container.exec(["uname", "-m"])
if exit_code == 0:
return arch_output.decode().strip()
return None
finally:
docker_utils.NotebookContainer(container).stop(timeout=0)
Option 2: Centralized Architecture Configuration
Create a configuration module for architecture-specific limitations:
# tests/containers/architecture_support.py
ARCHITECTURE_LIMITATIONS = {
"s390x": {
"pdf_export": False,
"reason": "TexLive and Pandoc dependencies not available"
}
}
def is_feature_supported(architecture: str, feature: str) -> bool:
return ARCHITECTURE_LIMITATIONS.get(architecture, {}).get(feature, True)
Option 3: Hybrid Approach
Combine both solutions for maximum efficiency and maintainability:
- Session-cached architecture detection
- Centralized feature limitation configuration
- Helper functions for common architecture checks
Acceptance Criteria
Core Requirements
- Eliminate redundant container start/stop cycles for architecture detection
- Create centralized configuration for architecture-specific feature limitations
- Implement helper functions for common architecture checks
- Maintain backward compatibility with existing test behavior
Performance Requirements
- Architecture detection should occur only once per test session
- Test execution time should be reduced compared to current implementation
- Container resource usage should be minimized
Maintainability Requirements
- Single authoritative source for architecture limitations
- Easy addition of new architecture-specific constraints
- Clear documentation of architecture support matrix
- Consistent architecture handling across all test methods
Testing Requirements
- Verify architecture detection works correctly on x86_64, arm64, and s390x
- Ensure PDF export tests still skip appropriately on s390x
- Confirm no regression in existing test functionality
- Test session caching behavior
Implementation Guidance
Phase 1: Helper Functions
- Create
tests/containers/architecture_utils.py
with detection helpers - Implement session-level caching for architecture detection
- Update
test_pdf_export
to use new helper functions
Phase 2: Centralized Configuration
- Create
tests/containers/architecture_support.py
configuration module - Define architecture limitation mappings
- Implement feature support checking functions
Phase 3: Integration and Testing
- Update all architecture-dependent tests to use new infrastructure
- Add comprehensive test coverage for architecture detection utilities
- Document architecture support matrix
Related Files
tests/containers/workbenches/jupyterlab/jupyterlab_test.py
(primary file)jupyter/utils/install_pdf_deps.sh
(s390x PDF limitation source)- Future test files that may need architecture-specific behavior
Context
- PR: Disable PDF export for s390x architecture in Jupyter Minimal images #1521 (Disable PDF export for s390x architecture in Jupyter Minimal images)
- Comment: Disable PDF export for s390x architecture in Jupyter Minimal images #1521 (comment)
- Requested by: @jiridanek
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Status
📋 Backlog