Added lint check for duplicate test ids

[engelmi]

Fixes: #2939

In order to be able detect duplicate ids of tests for a given tmt.Tree, a new lintable class Tests has been introduced. It doesn't reflect a specific node in the tree, but rather a collection of test nodes. On this collection, the duplicate lint check is being executed.

The lintable Tests implements the lint function for the duplicate test id check - this can be further extended in the future with additional lint checks.

Example usage triggering the new lint check:

$ tmt lint <path>
$ tmt tests lint <path>
$ tmt stories lint <path>
$ tmt plans lint <path>

# example output - detected no duplicate
Lint checks on all tests
pass G001 no duplicate test id detected

# example output - detected duplicate
Lint checks on all
fail G001 duplicate id "6886e7c8-7548-4cf7-8dd4-d2d46b125fd3" in test "/tests/clean/chain"
fail G001 duplicate id "6886e7c8-7548-4cf7-8dd4-d2d46b125fd3" in test "/tests/clean/runs"

Lint checks on all
fail G001 duplicate id "5cf92c54-e073-475c-970f-b8e090ec4da2" in "/duplicates/tests/duplicateid"
fail G001 duplicate id "5cf92c54-e073-475c-970f-b8e090ec4da2" in "/duplicates/tests/duplicateid"
fail G001 duplicate id "5cf92c54-e073-475c-970f-b8e090ec4da2" in "/duplicates/tests/duplicateid"

Pull Request Checklist

• implement the feature
• write the documentation
• extend the test coverage

[engelmi]

@happz I wanted to ask if this is going into the right direction as we discussed in the RFE #2939? (only a draft PR for now that implements the rough functionality)

[happz]

@happz I wanted to ask if this is going into the right direction as we discussed in the RFE #2939? (only a draft PR for now that implements the rough functionality)

In general, it looks good, it is indeed along the lines of some collection-wide linter. I'll add just a couple of minor minor comments.

Edit: BTW plans and stories also have IDs :)
Edit2: hm, can story have same id as a test? Maybe we should write this from the perspective of a collection of tmt objects, not just the collection of tmt tests. A container of not only tests, but any objects loaded by lint subcommand - tmt test lint would fill it with tests, while tmt lint would fill it with everything. tmt.base.Core instead of tmt.base.Test.

[engelmi]

In general, it looks good, it is indeed along the lines of some collection-wide linter. I'll add just a couple of minor minor comments.

Edit: BTW plans and stories also have IDs :) Edit2: hm, can story have same id as a test? Maybe we should write this from the perspective of a collection of tmt objects, not just the collection of tmt tests. A container of not only tests, but any objects loaded by lint subcommand - tmt test lint would fill it with tests, while tmt lint would fill it with everything. tmt.base.Core instead of tmt.base.Test.

Thanks for your feedback! I tried to apply the changes. PTAL @happz

I was also looking into updating the (unit) tests and it seems like when adding test files with the duplicate ids, the other tests will break because of this. What do you think about adding a flag to enable/disable such linters on collections?

[happz]

I was also looking into updating the (unit) tests and it seems like when adding test files with the duplicate ids, the other tests will break because of this. What do you think about adding a flag to enable/disable such linters on collections?

Did you try putting them into a dedicated, separate fmf root? We already have https://github.com/teemtee/tmt/tree/main/tests/lint/test/data, we could add something like data-duplicate-id or something, just for files to test this linter. Then tests for other linters would not consider these at all.

[engelmi]

Did you try putting them into a dedicated, separate fmf root? We already have https://github.com/teemtee/tmt/tree/main/tests/lint/test/data, we could add something like data-duplicate-id or something, just for files to test this linter. Then tests for other linters would not consider these at all.

Thanks for the hint! I think I was able to create a few new tests based on this.

[engelmi]

@happz Could you have another look at this PR? I've added tests now as well and think it should be complete.

[engelmi]

@happz Ping :)

[happz]

Tests are failing on testing-farm. I'll try to figure out what I broke and how to fix it. Do you have maybe any hints? @happz

Hmm, all seems unrelated to your changes, it seems there's some problem with debuginfo in rawhide, so, ruining all results.

[engelmi]

Tests are failing on testing-farm. I'll try to figure out what I broke and how to fix it. Do you have maybe any hints? @happz

Hmm, all seems unrelated to your changes, it seems there's some problem with debuginfo in rawhide, so, ruining all results.

Ah ok. In that case, can we re-run the pipeline again since a few days have passed?

[martinhoyer]

/packit build

[martinhoyer]

Nice. Maybe worth adding a release note.

[engelmi]

Nice. Maybe worth adding a release note.

Thanks! Should I add a release note to docs/releases.rst or is this done by the tmt maintainers on release? @martinhoyer

[martinhoyer]

Thanks! Should I add a release note to docs/releases.rst or is this done by the tmt maintainers on release? @martinhoyer

If you'll have time, feel free to add it there, thanks!
Oh, and if you're not yet in docs/overview.rst list of contributors, feel free to add yourself there, especially if you have any special name preference, otherwise we can add your name (from github account) during the release. :)

[engelmi]

Thanks! Should I add a release note to docs/releases.rst or is this done by the tmt maintainers on release? @martinhoyer

If you'll have time, feel free to add it there, thanks! Oh, and if you're not yet in docs/overview.rst list of contributors, feel free to add yourself there, especially if you have any special name preference, otherwise we can add your name (from github account) during the release. :)

Updated ✔️
@martinhoyer PTAL

[martinhoyer]

Updated ✔️ @martinhoyer PTAL

Thanks @engelmi, looks like you missed the release note in d06fd8e

[engelmi]

Updated ✔️ @martinhoyer PTAL

Thanks @engelmi, looks like you missed the release note in d06fd8e

Ups, good catch! Added it.

[martinhoyer]

/packit build

[martinhoyer]

/packit build

[happz]

/packit build