search

victorolinasc / junit-formatter

A JUnit XML report exporter for Elixir's ExUnit
https://hexdocs.pm/junit_formatter/
Apache License 2.0
111 stars 37 forks source link

Feature request: Automatic create report_dir #43

Closed wingyplus closed 3 years ago

wingyplus commented 3 years ago

Currently, we need to create a directory specified in report_dir if it doesn't exists. So my proposal is try creating that directory before generating reports.

Thoughts?

victorolinasc commented 3 years ago

Hi @wingyplus!

This has been brought up in the past too. I was avoiding adding this to the lib because it could create a directory anywhere on the system and that might open a plethora of other issues. For example, write permission, possible attacks in CI infrastructure (just guessing here) and so on.

Since it seems this is a very wanted feature perhaps we should just hide it in a very big warning + options. Not 100% sure about things here but generally I'd tend to walk the safest way.

What do you think?

wingyplus commented 3 years ago

Since it seems this is a very wanted feature perhaps we should just hide it in a very big warning + options. Not 100% sure about things here but generally I'd tend to walk the safest way.

Do you mean we should introduce a new configuration for creating directory and warn them if it's set right? If it's this case, I think it's good to try. :)

victorolinasc commented 3 years ago

Yep! That is what I thought. PRs welcome! If nobody steps up I might focus on this later on.

wingyplus commented 3 years ago

@victorolinasc I prototype PR here https://github.com/victorolinasc/junit-formatter/pull/44. Feels free to comment.

victorolinasc commented 3 years ago

Sorry for the delay... I'll be looking into it this week. I was thinking about checking if the file already exists... but it is on my todo list :)

wingyplus commented 3 years ago

Sorry for the delay... I'll be looking into it this week. I was thinking about checking if the file already exists... but it is on my todo list :)

I was thinking that we should cleanup report_dir before making a new one to make sure the report is correct result. What do you think?

victorolinasc commented 3 years ago

I think deleting files is always dangerous... that was the main issue about doing this in the first place =/

Imagine someone configures it badly and ends up deleting _build or deps or something even worse...

wingyplus commented 3 years ago

You're right. :)

So The only thing I can do with the PR might be check an error against mkdir_p and raise the error if it's not allow to create.