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.

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

Update tests #97

Merged
Merged

Conversation

SteffenBrinckmann
Copy link
Collaborator

@SteffenBrinckmann SteffenBrinckmann commented Nov 18, 2024

I updated the test:

Why have 4 tests: because they fulfill separate tasks:

  • 2 tests The ELN consortium cannot influence, we cannot specify our ro-crate dialect (tests: pypi-rocrate and validator)
  • the schema test allows us to use a schema to conveniently specify our dialect using data types
  • param-metadata test allows us to fine-tune the requirements of the consortium; like the hierarchy requirements mentioned in Hierarchical data structure in .eln or not #98.

Also it is more impressive if 4 tests are successful, than 1 or 2.

@nicobrandt
Copy link
Contributor

What about using https://github.com/crs4/rocrate-validator in addition to/instead of some of the checks? As we know, the library still has some issues but it seems to be steadily improving.

@SteffenBrinckmann
Copy link
Collaborator Author

Yes, good suggestion. I look into that

@SteffenBrinckmann
Copy link
Collaborator Author

I cannot get it to work, did it work for you @nicobrandt ?

@nicobrandt
Copy link
Contributor

We only tried it via the CLI so far. But I think programmatic validation should also work, it's just not well documented yet crs4/rocrate-validator#40

Where did you get stuck?

@FlorianRhiem
Copy link
Contributor

Maybe this snippet helps, it's how I've added rocrate_validator to SampleDB tests:

result = rocrate_validator.services.validate({
    "profiles_path": rocrate_validator.utils.get_profiles_path(),
    "profile_identifier": "ro-crate",
    "requirement_severity": rocrate_validator.models.Severity.REQUIRED.name,
    "requirement_severity_only": False,
    "inherit_profiles": True,
    "verbose": True,
    "data_path": rocrate_dir,
    "ontology_path": None,
    "abort_on_first": False
})
result_dict = result.to_dict()
assert result_dict['issues'] == []
assert result_dict['passed']

@SteffenBrinckmann
Copy link
Collaborator Author

"rocrate-validator validate examples/kadi4mat/collections-example.zip"
leads to "[FAILED] RO-Crate is not a valid ro-crate-1.1 !!!" which I totally do not agree with in its entirety.

@SteffenBrinckmann
Copy link
Collaborator Author

@FlorianRhiem can you write a test for it in the tests directory?

@FlorianRhiem
Copy link
Contributor

"rocrate-validator validate examples/kadi4mat/collections-example.zip" leads to "[FAILED] RO-Crate is not a valid ro-crate-1.1 !!!" which I totally do not agree with in its entirety.

I just ran the code from my post above with the unzipped collections-example from PR #100 and it passes. Maybe the issue is that it's the old .eln file or that it needs to be unzipped? If that doesn't help, I can look into writing a test tomorrow.

@nicobrandt
Copy link
Contributor

Well our current example is indeed not valid :P You would have to try with the fixed ones in #100

@nicobrandt
Copy link
Contributor

Actually, that just reminded me that we also had trouble using the CLI on zipped files, even when using minimal examples, while the same content was valid after unzipping. @jmanideep wanted to create an issue for that in the validator repo.

@SteffenBrinckmann
Copy link
Collaborator Author

That was the issue: that thing cannot read into zip files.
I wrote the first version of such a test and will likely publish it later today, if time permits.

@SteffenBrinckmann
Copy link
Collaborator Author

OK, it works as intended; the latest SampleDB and Kadi4Mat examples pass all tests.
I updated the description at the very top of this PR

@jmanideep
Copy link
Contributor

An issue has been created in the roc-validator repository regarding validate CLI command not working for validating ZIP files. Details can be found here: Issue link

Copy link
Contributor

@NicolasCARPi NicolasCARPi left a comment

Choose a reason for hiding this comment

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

👍

@SteffenBrinckmann SteffenBrinckmann merged commit 8cd9737 into TheELNConsortium:master Nov 20, 2024
1 check passed
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.

5 participants