teemtee / fmf

Flexible Metadata Format
GNU General Public License v2.0
22 stars 28 forks source link

Add directive `select` to force selecting node #223

Closed lukaszachy closed 3 months ago

lukaszachy commented 6 months ago
lukaszachy commented 6 months ago

Limiting tests in #225. Lets see

jscotka commented 5 months ago

Hi, I would love some more descriptive name, instead of is-leaf e.g. someghing like marked-as_leaf, or I can imagine something underscoder, to avoid confusion user data and TMT will then ignore validation these attributes, like: _mark_as_leaf: True and in TMT there could be rule to skip underscored items, as they are directives and should not be used as metadata source directly. But be backward compatible, and display it to users as well

happz commented 5 months ago

... or I can imagine something underscoder, to avoid confusion user data and TMT will then ignore validation these attributes, like: _mark_as_leaf: True and in TMT there could be rule to skip underscored items

tmt can easily ignore directives with dashes simply by listing them in tmt sources, if needed, plus users may define their custom keys with underscores in their tests (extra-foo_bar, https://github.com/teemtee/tmt/blob/main/tmt/schemas/test.yaml#L128), therefore a rule as general as "ignore everything with underscores" might cause more harm than good.

I suspect these directives don't even reach tmt, do they? They are consumed by fmf, applied and tmt gets the final materialized tree of data, correct?

LecrisUT commented 5 months ago

What about also having the opposite? I.e. marking a file.fmf as non-leaf. I was thinking if this might be necessary in the context of: https://github.com/teemtee/fmf/blob/29190c78e4065adf98cbb3ccf747b01973d761e2/fmf/base.py#L812-L815


More specifically, in https://github.com/LecrisUT/tmt-copr/issues/2 I want discover plugin to inject main.fmf-like file into:

/prefix:
  framework: copr
  test: custom-placeholder
/prefix/pkg1:
  package: pkg1

Then there could be overrides defined in /prefix/pkg1.fmf like

result: xfail

The idea of adding a function to mark something as non-leaf is if we only have a /prefix.fmf file:

duration: 10h

In this case tmt lint will fail because /prefix.fmf is neither a plan, nor a test. It would be nice to mark this as being non-leaf so that it is not parsed by tmt lint (or at least not before discover is run) or other phases.

lukaszachy commented 5 months ago

I suspect these directives don't even reach tmt, do they? They are consumed by fmf, applied and tmt gets the final materialized tree of data, correct?

Correct.

lukaszachy commented 5 months ago

What about also having the opposite?

I see no problem - mark-non-leaf as the name for the directive? If both are set it will result with fmf.utils.FormatError

jscotka commented 5 months ago

I suspect these directives don't even reach tmt, do they? They are consumed by fmf, applied and tmt gets the final materialized tree of data, correct?

Correct.

This is question. On first singht, I can imagine that TMT will interpret the value, but another is, that it is semantic of FMF, so that it should be materialized and TMT will do not see this key anyway. as @happz mentioned, the underscored semantics could cause harm in case of ignoring them. In opposite side, when someone using is-leaf attribute today. it will break someone workflow in future, so that Yes, This underscoring is not good, But I would like to see also some special mark to intepret these values, e.g. long time ago, we've discussed of using ./: or something similar as node name path. so that I can imagine also sometghing like:

this_is_leaf/:
  ./:
    mark_as_leaf: True
    another_fmf_firective: something
  test: shell.sh

BTW, this disussion is closer to FMF/TMT config/profiles (https://github.com/teemtee/fmf/pull/196) than we should allow :-), it allows you to configure/override default FMF behaviour, what do you think? From one perspective it is very simple PR, from another perspective overriding, redefining nodes or change some behaviour seems to me very close to profiles and configs, that's Why I would like to see something more generic, than just one attribute to change befaviour of FMF, but do it more extensible.

lukaszachy commented 5 months ago

This is question. On first singht, I can imagine that TMT will interpret the value, but another is, that it is semantic of FMF, so that it should be materialized and TMT will do not see this key anyway.

tmt doesn't see this value, similar to merge suffixes (+, -, +<), all of this is processed by fmf.

lukaszachy commented 5 months ago

BTW, this disussion is closer to FMF/TMT config/profiles (#196) than we should allow :-), it allows you to configure/override default FMF behaviour,

I beg do differ - for me this is just setting fmf directive. By overriding default fmf behavour I'd expect custom _merge_special, injection of directives not known to fmf or similar.

lukaszachy commented 5 months ago

Hm, I'll leave the mark-non-leaf feature to the future. As it changes the flow of creating the tree a lot -- if I understood correctly such nodes will be effectively 'main.fmf' -> not sure what to do if more are present, which data win...

LecrisUT commented 5 months ago

not sure what to do if more are present

Not sure what you mean here. It would have the same inheritance rules, i.e. anything within that file is marked as non-leaf. I was thinking of just adding a special case of Tree.children=dict(None: None) or something similar, and guard inside the generators to not export that, e.g.: https://github.com/teemtee/fmf/blob/29190c78e4065adf98cbb3ccf747b01973d761e2/fmf/base.py#L573-L579

lukaszachy commented 5 months ago

Could you please create a new issue, @LecrisUT ?

What I have in mind is this:

.
├── bar
│   └── main.fmf
├── .fmf
│   └── version
├── foo.fmf
└── main

Currently this produces leaf /, non-leaves /foo and /bar. Inheritance works in way that /bar is created based on 'main.fmf' and 'bar/main.fmf'.

If foo.fmf is 'mark-non-leaf' - how it will ever become a leaf aka node usable by tmt? Should it become parent of some none, similar to the 'main.fmf'?

LecrisUT commented 5 months ago

Currently this produces leaf /, non-leaves /foo and /bar.

You mean the opposite right? non-leaf / and /foo, /bar are leaves (in the current implementation, and discussion does not touch the current implementation right?)

If foo.fmf is 'mark-non-leaf' - how it will ever become a leaf aka node usable by tmt? Should it become parent of some none, similar to the 'main.fmf'?

The idea is to support the inverse of grafting where the base is populated by discover and on top of that we apply fmf.Tree from filesystem:

But I guess in both cases you could have breaking tmt lint due to incomplete data, so maybe it only needs to be on the tmt side (other than graft/inverse_graft support on fmf side)

lukaszachy commented 4 months ago

You mean the opposite right? non-leaf / and /foo, /bar are leaves (in the current implementation, and discussion does not touch the current implementation right?)

Right you are, I flipped those two terms.

lukaszachy commented 4 months ago

PR rebased, in the end I went with 'select' as directive name as it made more sense in the docs.