Codecov Report

Merging #972 into dev will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev     #972      +/-   ##
==========================================
- Coverage   53.58%   53.56%   -0.02%     
==========================================
  Files         129      129              
  Lines       18383    18383              
  Branches     4407     4407              
==========================================
- Hits         9850     9847       -3     
  Misses       5822     5822              
- Partials     2711     2714       +3     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b979704...728c818. Read the comment docs.

FYI, @stevenengler from our dev team will be helping move this along :)

Once you're ready for me to take a look, just mark me as a reviewer (I'm not sure if you're still making changes).

@stevenengler yes please, you could. Don't hesitate to check the recent comment of @robgjansen on the #773 to be sure we all follow the same idea.
I let the "WIP", because I will work on other tests once you will have validated this first step.

@stevenengler everything looks good to you ? Can I spread this method on others tests ?

@stevenengler everything looks good to you ? Can I spread this method on others tests ?

Looks good to me, thanks for making the changes! And yep, adding this to the other tests sounds good.

If you haven't started on the src/test/socket/* tests, you can leave those and I can do them later since I'm currently modifying them. (If you've already started them that's fine too.)

@stevenengler ok I will ignore them

@sporksmith I let src/test/phold/phold.test.shadow.config.xml and src/test/file/file.test.shadow.config.xml because there is a commentary in them. Should I update the script to take care of the comment or should I remove them or put them in another place ?

@stevenengler , could you give me a hand with the centos7 error ?

@florian-vuillemot: it looks like the xml2yaml script isn't converting the environment attribute on the shadow element.

Well, I will pay more attention the next time I will check the conversion.. Thank you !

No problem, is this a bug in the script that we should fix? I took a quick look but I don't see an obvious reason why it would skip that attribute.

@sporksmith I let src/test/phold/phold.test.shadow.config.xml and src/test/file/file.test.shadow.config.xml because there is a commentary in them. Should I update the script to take care of the comment or should I remove them or put them in another place ?

Updating the script to preserve comments would be a nice feature, though manually recreating the comments would also be ok.

@stevenengler after having tested the convert.py script I think I might have forgotten to run the script on the new (merged) XML file with the environment option ^^

@sporksmith I let src/test/phold/phold.test.shadow.config.xml and src/test/file/file.test.shadow.config.xml because there is a commentary in them. Should I update the script to take care of the comment or should I remove them or put them in another place ?

Updating the script to preserve comments would be a nice feature, though manually recreating the comments would also be ok.

I just copy/past commentaries because it is quick, simple and I don't think we will have a huge amount of files with commentaries, so automation does not look mandatory. If I'm wrong, don't hesitate to open an issue and assign it to me.

FYI, yesterday I merged another test in src/test/futex that should probably also be converted. Sorry!

I also just noticed that there are two other directories that need to be converted, src/test/tcp and src/test/poll. Some of these are currently commented out, but I hope to re-enable them soon, so they would be good to convert.

Thanks @florian-vuillemot for the changes! It's great to be another step closer to full yaml support. I've checked the box at #773 (comment) since I'm not sure which accounts are allowed to edit comments by other accounts.

Woo! Thanks for all the work to get this merged! 🥇