Skip to content

Conversation

@o-muzyka
Copy link

@o-muzyka o-muzyka commented Oct 17, 2025

Description

Introduces AdaptiveGraph to centralize SAMM version management, with automated detection, namespace validation, and CLI‑driven upgrades that eliminate version mismatches across the codebase.

  • Core: Added adaptive_graph.py with AdaptiveGraph (wraps rdflib.Graph, auto-detects SAMM version, validates, upgrades via SAMM CLI prettyprint).
  • Validation: Added a _validate_samm_namespace_version method to SAMMGraph that checks RDF graph namespaces for SAMM version consistency and raises a ValueError if a mismatch is detected.
  • CI: Updated workflow to install and run download-samm-cli for automated upgrades in builds/tests.
  • Tests: unit + integration + upgraded to version 2.2.0

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works

@o-muzyka o-muzyka force-pushed the feature/check-samm-version branch 7 times, most recently from 2d2bee1 to 8f2eea9 Compare October 23, 2025 22:02
@o-muzyka o-muzyka force-pushed the feature/check-samm-version branch 6 times, most recently from 5e310da to 73494a9 Compare October 29, 2025 20:39
@o-muzyka o-muzyka marked this pull request as ready for review October 29, 2025 20:54
@o-muzyka o-muzyka changed the title Add SAMM version validation to SAMMGraph load_aspect_model Auto‑Detect, Validate & Upgrade SAMM files via CLI prettyprint Oct 30, 2025
@o-muzyka o-muzyka changed the title Auto‑Detect, Validate & Upgrade SAMM files via CLI prettyprint Detect, Validate & Upgrade SAMM files via CLI prettyprint Oct 30, 2025
@o-muzyka o-muzyka force-pushed the feature/check-samm-version branch 2 times, most recently from 286a277 to 469339d Compare October 30, 2025 01:31
@o-muzyka o-muzyka marked this pull request as draft November 4, 2025 09:52
@o-muzyka o-muzyka force-pushed the feature/check-samm-version branch 2 times, most recently from 703e820 to cc4a1d6 Compare November 10, 2025 19:51
- Enhance SAMM CLI functions with capture output option and update tests
- Add ability to download samm-cli for macOS
- Add archive extraction functionality to download_samm_cli
- Add SAMM CLI installation and native dependencies to CI workflow
- Stop running tests after the first failure
- Refactor resource path in test_cli_functions to improve clarity and maintainability
- Refactor SAMM CLI path retrieval for cross-platform compatibility
@o-muzyka o-muzyka force-pushed the feature/check-samm-version branch 7 times, most recently from 9cb09a8 to fe467ed Compare November 13, 2025 02:04
@o-muzyka o-muzyka force-pushed the feature/check-samm-version branch 3 times, most recently from 88ce235 to c888b97 Compare November 13, 2025 16:21
@o-muzyka o-muzyka marked this pull request as ready for review November 21, 2025 07:00
model_element_factory_mock.create_element.assert_called_once_with("aspect_urn")
validate_samm_namespace_version_mock.assert_called_once_with("rdf_graph_samm_graph")

class TestValidateSammNamespaceVersion:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need a separate class for testing one method?

Copy link
Author

@o-muzyka o-muzyka Dec 4, 2025

Choose a reason for hiding this comment

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

I see your point, but I chose a class here mainly for three reasons:

  1. Common setup / DRY principle
    Using a test class allows me to apply the @mock.patch decorator once at the class level, so that
    get_samm_versions_from_graph_mock is injected into all test methods without repeating the patch in each one.

  2. Naming clarity
    Instead of long method names like
    test_validate_samm_namespace_version_valid / test_validate_samm_namespace_version_mismatch,
    the class name already scopes the tests to _validate_samm_namespace_version.
    Inside the class, method names can then be short and meaningful like test_valid and test_mismatch.

  3. Future expansion / Better organization for related logic
    Basically, it keeps all related tests grouped, which simplifies maintenance and extends naturally if the class grows.
    If common mocks, fixtures, or shared setup become relevant to more than one test, a class-based approach makes them easier to share and keep consistent.
    With a class, I can run all the related tests together (e.g., pytest ::TestValidateSammNamespaceVersion) without having to run each function separately.
    This makes running/debugging focused test groups much easier.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok
Agrred with these arguments

Let's just place the code for the new TestValidateSammNamespaceVersion class after TestSAMMGraph

return self._upgrade_ttl_file(source_path)

def _upgrade_data(self, data: str | bytes) -> str:
print( # TODO: improve logging
Copy link
Contributor

Choose a reason for hiding this comment

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

Imact 0:
You can create a task in backlog to analyze and implement a logging.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@atextor atextor left a comment

Choose a reason for hiding this comment

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

LGTM!

Oleksandr Muzyka (EPAM) added 3 commits December 5, 2025 10:57
@o-muzyka o-muzyka force-pushed the feature/check-samm-version branch from b5f424d to a4f9f59 Compare December 5, 2025 09:58
@atextor atextor merged commit 93d51f6 into eclipse-esmf:main Dec 5, 2025
2 checks passed
@atextor atextor deleted the feature/check-samm-version branch December 5, 2025 10:39
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