Added tests cases for covering metadata type mismatching

[kbushgit]

Signed-off-by: Kostiantyn Bushko kbushko@intellias.com Test case for metadata type mismatching: Testing Director and Image repositories with all defined metadata types.

[pattivacek]

Cool, thanks! This looks just about right. You are right about the inconsistency with the exception type that you mentioned earlier. I think if possible you should try to fix libaktualizr to consistently throw InvalidMetadata for these kinds of errors. You may have to work on this PR and one on libaktualizr in parallel.

Did you use a tool to reformat the python code? We have a ticket for adding a CI for tuf-test-vectors and we should think about enforcing a formatting standard as part of that tooling.

[kbushgit]

Cool, thanks! This looks just about right. You are right about the inconsistency with the exception type that you mentioned earlier. I think if possible you should try to fix libaktualizr to consistently throw InvalidMetadata for these kinds of errors. You may have to work on this PR and one on libaktualizr in parallel.

Did you use a tool to reformat the python code? We have a ticket for adding a CI for tuf-test-vectors and we should think about enforcing a formatting standard as part of that tooling.

Yes, if we change libaktualizr to be able throws with InvalidMetadata for this kind of error, it will also require to make minor changes in the uptane.py file.

To be able to follow PEP8 standard I'm using PyCharm IDE, this ide can automatically reformat the python file, it's quite impossible to remember all requirements from PEP8 and do it without a helper, I prefer PyCharm

[lbonn]

Did you use a tool to reformat the python code? We have a ticket for adding a CI for tuf-test-vectors and we should think about enforcing a formatting standard as part of that tooling.

To be able to follow PEP8 standard I'm using PyCharm IDE, this ide can automatically reformat the python file, it's quite impossible to remember all requirements from PEP8 and do it without a helper, I prefer PyCharm

If we'd like to follow an approach similar to our policy on aktualizr where there is only one good formatting, we might consider integrating one of the popular auto-formatters (like https://github.com/python/black which is way stricter than just pep8) and integrate them on CI.

Might be overkill for such a small project though?

[kbushgit]

Did you use a tool to reformat the python code? We have a ticket for adding a CI for tuf-test-vectors and we should think about enforcing a formatting standard as part of that tooling.

To be able to follow PEP8 standard I'm using PyCharm IDE, this ide can automatically reformat the python file, it's quite impossible to remember all requirements from PEP8 and do it without a helper, I prefer PyCharm

If we'd like to follow an approach similar to our policy on aktualizr where there is only one good formatting, we might consider integrating one of the popular auto-formatters (like https://github.com/python/black which is way stricter than just pep8) and integrate them on CI.

Might be overkill for such a small project though?

Yes, I agree, this it is a relatively small project but in the same time he is a part of big enough project, and since in our big solution we use different languages, maybe for that reason we should have more approaches for checking code? It's all about code quality.

[pattivacek]

If we'd like to follow an approach similar to our policy on aktualizr where there is only one good formatting, we might consider integrating one of the popular auto-formatters (like https://github.com/python/black which is way stricter than just pep8) and integrate them on CI. Might be overkill for such a small project though?

Yes, I agree, this it is a relatively small project but in the same time he is a part of big enough project, and since in our big solution we use different languages, maybe for that reason we should have more approaches for checking code? It's all about code quality.

I'm in favor of picking a tool and integrating it into the repository. Separate PR, and not necessarily a high priority, but worth doing. Ideally, we should use the same thing across all python repositories. I think our QA team (@raigi and @epicnuts) are the most likely to have a shared interest.

[pattivacek]

@kbushgit Could you separate this PR into two PRs, one for formatting with a reliable tool, and one for the actual feature here? It also needs to get rebased on the latest master.

[kbushgit]

@kbushgit Could you separate this PR into two PRs, one for formatting with a reliable tool, and one for the actual feature here? It also needs to get rebased on the latest master.

Can we make refactoring regarding PEP8 after merging this PR to master?

[lbonn]

I'd recommend using black and integrating it into CI and can propose a PR, if you don't insist on doing so. It works well in my experience and leaves no room for interpretation.

[kbushgit]

I just checked the latest changes on master for aktualizr and tuf-test-vector and I got this result...

[ FAILED ] 7 tests, listed below: [ FAILED ] UptaneVectorSuite/UptaneVector.Test/2, where GetParam() = "delegation_empty" [ FAILED ] UptaneVectorSuite/UptaneVector.Test/3, where GetParam() = "delegation_expired" [ FAILED ] UptaneVectorSuite/UptaneVector.Test/4, where GetParam() = "delegation_hash_mismatch" [ FAILED ] UptaneVectorSuite/UptaneVector.Test/8, where GetParam() = "delegation_path_mismatch" [ FAILED ] UptaneVectorSuite/UptaneVector.Test/28, where GetParam() = "director_target_hash_mismatch" [ FAILED ] UptaneVectorSuite/UptaneVector.Test/35, where GetParam() = "image_repo_bad_hw_id" [ FAILED ] UptaneVectorSuite/UptaneVector.Test/52, where GetParam() = "image_repo_target_hash_mismatch"

[pattivacek]

@kbushgit Try it with my latest PR: https://github.com/advancedtelematic/aktualizr/pull/1258. They've been fixed there.