Skip to content

Conversation

@GernotMaier
Copy link
Contributor

Six integration tests are skipped due to a bug in the model version comparison. This PR fixes this.

  • reason for skipping is the recent introduction of being able to avoid patch versions. The comparison of 'request version for tests = 6.0' with the 'actual version used = 6.0.3' (example) did not work correctly. This has been fixed by introducing the possibility to compare version strings for different levels.

Required to fix an additional issue completely unrelated (but which led to failed unit tests):

  • model_parameter.get_parameter_value_with_unit() did not work correctly for mixed lists of values / units (e.g. value = [5., "abc"] and unit = ["cm", None]

@GernotMaier GernotMaier self-assigned this Sep 30, 2025
@GernotMaier GernotMaier marked this pull request as ready for review September 30, 2025 10:41
@GernotMaier GernotMaier requested a review from Copilot September 30, 2025 13:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes six skipped integration tests by resolving a version comparison bug that occurred when comparing versions with different precision levels (e.g., "6.0" vs "6.0.3"). Additionally, it addresses an unrelated issue in model parameter handling for mixed value/unit lists.

  • Introduces new version comparison functions that can handle different version precision levels
  • Fixes model parameter value retrieval for mixed lists containing both values and units
  • Updates integration test configurations to use consistent version formats

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/simtools/version.py Adds version comparison functions with configurable precision levels
src/simtools/testing/configuration.py Updates test skipping logic to use new version comparison
src/simtools/model/model_parameter.py Fixes parameter value handling for mixed value/unit lists
tests/unit_tests/test_version.py Adds comprehensive tests for new version comparison functions
tests/unit_tests/model/test_model_parameter.py Adds test for the new quantity creation helper method
tests/integration_tests/config/*.yml Updates version format consistency
src/simtools/simtel/simulator_light_emission.py Removes label parameter from output directory call
docs/changes/1805.bugfix.md Documents the bug fixes

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@ctao-sonarqube
Copy link

Copy link
Collaborator

@tobiaskleiner tobiaskleiner left a comment

Choose a reason for hiding this comment

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

Thanks @GernotMaier , all good.

@GernotMaier
Copy link
Contributor Author

Thanks @tobiaskleiner

@GernotMaier GernotMaier merged commit 24f29b4 into main Oct 1, 2025
12 checks passed
@GernotMaier GernotMaier deleted the skipped-integration-tests branch October 1, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants