Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Reuse JAVA SDK test suite #39

Merged
merged 2 commits into from
Jun 25, 2024
Merged

Reuse JAVA SDK test suite #39

merged 2 commits into from
Jun 25, 2024

Conversation

Hanna-Shalamitskaya-EPAM
Copy link
Contributor

@Hanna-Shalamitskaya-EPAM Hanna-Shalamitskaya-EPAM commented Jun 4, 2024 •
edited
Loading

Description

Added a script for download test models and test for loading models.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

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

@Hanna-Shalamitskaya-EPAM Hanna-Shalamitskaya-EPAM force-pushed the reuse-java-sdk-test-suite branch 2 times, most recently from 7632a12 to 1895daa Compare June 5, 2024 05:19
atextor
atextor requested changes Jun 5, 2024
View reviewed changes
core/esmf-aspect-meta-model-python/scripts/download_test_models.py Outdated

FOLDER_TO_EXTRACT = "valid"
TEST_MODELS_PATH = join("tests", "integration", "java_models", "resources")
VERSION = "2.7.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

In download_samm_cli.py there is another constant, SAMM_CLI_VERSION (which currently points to a different esmf-sdk version!). There should be only one place where the esmf-sdk version is defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Move all constants to the separate file

atextor
atextor requested changes Jun 10, 2024
View reviewed changes
core/esmf-aspect-meta-model-python/README.md Outdated
- [Install project dependencies](#install-project-dependencies)
- [Download SAMM files](#download-samm-files)
Optional
- [Download SAMM CLI](#download-samm-cli) (need to use SAMM CLI functions)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- [Download SAMM CLI](#download-samm-cli) (need to use SAMM CLI functions)
- [Download SAMM CLI](#download-samm-cli) (needed to use SAMM CLI functions)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

core/esmf-aspect-meta-model-python/README.md

In order to download the SAMM sources, it is required to run `poetry install` once in the `esmf-aspect-model-loader`
module. There are two possibilities to download the SAMM files and extract the Turtle sources for the Meta Model.
There are two possibilities to download the SAMM files and extract the Turtle sources for the Meta Model:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this section should also explain for whom are the SAMM files interesting, or in other words, why would you want to download them. IMO users of the aspect-meta-model-python should not have to deal with this at all (if they don't want to), including that they should not need to know about this.

core/esmf-aspect-meta-model-python/README.md Outdated
The script uses the GitHub API and downloads the files from the `main` branch. If the script is run in a
pipeline, it uses a GitHub token to authorize. If the script is run locally, the API is called without a token.
This may cause problems because unauthorized API calls are limited.
### Download SAMM CLI
Copy link
Contributor

Choose a reason for hiding this comment

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

This section I don't understand, shouldn't this work transparently? I.e., users should not be required to know about this at all, it should "just work". samm-cli should be downloaded automatically "under the hood" when it's not present yet.

core/esmf-aspect-meta-model-python/README.md Outdated
```
poetry run download-samm-branch

How to work with SAMM CLI from the Aspect Model Loader you can read in the chapter [SMM ClI wrapper](#samm-cli-wrapper).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
How to work with SAMM CLI from the Aspect Model Loader you can read in the chapter [SMM ClI wrapper](#samm-cli-wrapper).
How to work with SAMM CLI from the Aspect Model Loader you can read in the chapter [SAMM CLI wrapper](#samm-cli-wrapper).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

core/esmf-aspect-meta-model-python/README.md Outdated
More detailed information about SAMM CLI functionality can be found in the
[SAMM CLI documentation](https://eclipse-esmf.github.io/esmf-developer-guide/tooling-guide/samm-cli.html).

### Download test models
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this also does not belong in the README. Shouldn't this be done automatically when the tests are run?

core/esmf-aspect-meta-model-python/README.md Outdated

The SAMM CLI is a command line tool provided number of functions for working with Aspect Models.

ore detailed information about SAMM CLI functionality can be found in the
Copy link
Contributor

Choose a reason for hiding this comment

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

This paragraph is missing the start of a sentence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

core/esmf-aspect-meta-model-python/README.md Outdated
ore detailed information about SAMM CLI functionality can be found in the
[SAMM CLI documentation](https://eclipse-esmf.github.io/esmf-developer-guide/tooling-guide/samm-cli.html).

Python aspect Model Loader provide a wrapper class to be able to call SAMM CLI functions from the python code.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Python aspect Model Loader provide a wrapper class to be able to call SAMM CLI functions from the python code.
Python Aspect Model Loader provide a wrapper class to be able to call SAMM CLI functions from the Python code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

core/esmf-aspect-meta-model-python/README.md Outdated
[SAMM CLI documentation](https://eclipse-esmf.github.io/esmf-developer-guide/tooling-guide/samm-cli.html).

Python aspect Model Loader provide a wrapper class to be able to call SAMM CLI functions from the python code.
For instance, validation a model can be done with the next code snippet
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For instance, validation a model can be done with the next code snippet
For instance, validation of a model can be done with the following code snippet:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

core/esmf-aspect-meta-model-python/README.md Outdated
from esmf_aspect_meta_model_python.samm_cli_functions import SammCli

samm_cli = SammCli()
model_path = "Paht_to_the_model//Model.ttl"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
model_path = "Paht_to_the_model//Model.ttl"
model_path = "Path_to_the_model/Model.ttl"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@Hanna-Shalamitskaya-EPAM Hanna-Shalamitskaya-EPAM force-pushed the reuse-java-sdk-test-suite branch 2 times, most recently from 66da704 to 5bed60f Compare June 24, 2024 13:20
@atextor atextor merged commit 10a4af1 into eclipse-esmf:main Jun 25, 2024
2 checks passed
@atextor atextor deleted the reuse-java-sdk-test-suite branch June 25, 2024 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yauhenikapl Yauhenikapl Yauhenikapl approved these changes

@atextor atextor atextor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Hanna-Shalamitskaya-EPAM @atextor @Yauhenikapl