-
Notifications
You must be signed in to change notification settings - Fork 28
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
Ignore metadata subtrees #43
Comments
I like the solution we discussed some time ago: placing all trees to the same level (and creating forest by doing so). So the output in the main directory would look like this:
This is great for merging downstream and upstream, where you can place one of them in subdirectory of the other and the metadata will seamlessly merge together. Usage of this is for example keeping credentials downstream but the test itself upstream. Another approach is to have
This has the same advantages / disadvantages as above, just stronger. It lets the user say if he wants to merge it and breaks the path_in_filesystem for everything if the name is different for the main directory too. Third way could be having 'switch' in .fmf directory to say if we want to merge the subtrees or ignore them. This is probably the smallest change of these 3 and easiest to do. Something like Fourth way is not giving a choice at all and ignoring them right away... For now I am in favour of the last approach, because it is the smallest one, gives user choice and does not limit implementing one of the first 2 in the future (I like the 2nd better). This has only one issue: Implementing one of the first 2 would introduce a breaking change. Hope that helps :) |
I think good practice is to mimic model of filesystem, ie. file/directory. Then looking at how common utilities work could answer some questions. For example:
|
Hi,
I don't know why, but I found this the most intuitive for me :-) |
@AloisMahdal By credentials I mean user / pass to server and its address. For example I have test that tests a thing. It runs on already provisioned clean server, but it has to connect to different servers to do its thing. Some of these servers are downstream (private), some upstream. It does not make much sense to copy this test multiple times and split it to upstream / downstream test, as the code is the same and only the credentials change. IMO much better approach is let the layer above handle it (e.g. test suite) by providing this metadata to the test and running it multiple times with different metadata. Adding new server to this testing is just matter of adding single leaf to FMF, no code change needed. Basically I use metadata to not only describe the test but also to give it some values to alter its behaviour. So the test is defined by not only its code, but also its metadata.
So I would prefer to see it like this:
But for this to happen we need much broader discussion and implement some sort of metadata of metadata, so user knows where each attribute value came from. Other nice examples of this metadata is type of the attribute, its description... Regarding the #26: its point was to have the same metadata no matter where you are in the tree, not alter merging. |
Guys, thank you very much for your detailed opinions and many good points. I've read the comments carefully and here's my summary:
I think that the solution should definitely align with the following statement:
That's why finding the top-most |
When exploring directories
fmf
should probably ignore other metadata trees in the directory tree. Yesterday we've discussed the following real-life scenario which seems to support this approach:Standard Test Roles support fetching tests from remote repositories. At the same time there can be tests stored directly in dist-git. When these two options are mixed we should still be able to gather reasonable metadata for both.
Here's a simple example directory structure:
Current output in the main
tests
directory (note the modified node identifiers):Current output for the fetched test cases in
tests/fetched
:Expected output for the main directory:
Guys, what are your thoughts on this?
The text was updated successfully, but these errors were encountered: