teemtee / tmt

Test Management Tool
MIT License
80 stars 122 forks source link

'ID' might be exported but not stored in metadata #1503

Open lukaszachy opened 2 years ago

lukaszachy commented 2 years ago

I believe we have problem with generating 'id' as part of the test export (to polarion/nitrate). It is very easy to export & forget that git was modified so the only place where 'id' is stored is TCMS/polarion. (or plain git reset --hard to get rid of unwanted modifications)

If it is required then it should be created before AND committed to the git (same as other metadata changes). If it is optional then do not create it as part of the export without explicit request.

Thoughts?

@psss @KwisatzHaderach @hegerj

hegerj commented 2 years ago

Personally I ignore id changes. I don't understand what it is for (even after asking), so I do not want it as a part of my tests. Therefore I am strongly for the second option.

KwisatzHaderach commented 2 years ago

IMO the id should be required, but it's not entirely necessary, it's used mainly for exporting testruns to polarion for case searching (it's more precise than the other 2 methods: tcms case id or beaker-task)

This is literally the same as with extra-nitrate field, export generates it but it's not necessary right? well, no, another export would fail when extra-nitrate field is not there and yet there is no forcing the user to git push

btw, on Friday I submitted a PR for import https://github.com/teemtee/tmt/pull/1502 which could pull the ID from Polarion, so the ID might not be lost to time (and git resets)

hegerj commented 2 years ago

This is literally the same as with extra-nitrate field, export generates it but it's not necessary right? well, no, another export would fail when extra-nitrate field is not there and yet there is no forcing the user to git push

No. extra-* fields are generated during import and it is forced that they are pushed before export.

KwisatzHaderach commented 2 years ago

I've seen the code and done multiple imports, but am very interested where/how is this enforced.

hegerj commented 2 years ago

By the enforcing I meant tmt.utils.validate_git_status(), which doesn't check extra-* specifically, but it checks that the imported tests were pushed before export.

KwisatzHaderach commented 2 years ago

Exactly, so it works the same for id parameter.

hegerj commented 2 years ago

Yes but the difference is that all other parameters are generated during import so you are forced to push it if you want to export, but id is generated during export when I don't really care about it.

KwisatzHaderach commented 2 years ago

Well, with the PR I posted it will be generated during import as well.

Can add the generation for TCMS import if it will be required, but considering TCMS is supposed to die at EOY, I don't think it's that important.

hegerj commented 2 years ago

I am still not convinced generating id by default is correct thing to do for our tests. There is no benefit and it causes confusion.