Skip to content
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

HTML report produces html files in the wrong directory #521

Closed
jcamiel opened this issue Mar 28, 2022 · 9 comments
Closed

HTML report produces html files in the wrong directory #521

jcamiel opened this issue Mar 28, 2022 · 9 comments
Labels
bug Something isn't working
Milestone

Comments

@jcamiel
Copy link
Collaborator

jcamiel commented Mar 28, 2022

Given some Hurl files in /tmp/dir_a/*.hurl the following command:

hurl --report-html /tmp/dir_b --test /tmp/dir_a/*.hurl will produces html file for each Hurl file in /tmp/dir_a/.

Not sure if it is by design, but it does seem wrong to me.

Before execution:

$ find /tmp/dir_a
/tmp/dir_a
/tmp/dir_a/test.hurl

After execution:

$ find /tmp/dir_a
/tmp/dir_a
/tmp/dir_a/test.hurl
/tmp/dir_a/test.hurl.html
@jcamiel jcamiel added the bug Something isn't working label Mar 28, 2022
@fabricereix
Copy link
Collaborator

This is indeed not the expected behavior.
All the generated files should always be written within the directory specified by --report-html, whatever the Hurl input files are specified with a relatively or absolute path.

@fabricereix fabricereix added this to the 1.7.0 milestone Mar 30, 2022
@jleverenz
Copy link
Contributor

I'd be happy to take a stab at a PR to address this, but have some questions about the desired functionality.

Currently, the "input file html" files for each input file looks to be created next to the original input file. This is the extra html file in the dir_a directory of the example. Pretty straightforward to create the "input file html" file in the output report directory instead. However, there is potential for name collision, e.g. with input files "/tmp/dir_a/test.hurl" and "./test.hurl". My first thought is to use a hash for each input file. Output directory would then be something like:

% find /tmp/dir_b
/tmp/dir_b
/tmp/dir_b/55eb9791cbcf2ca772ce3a6c36900295.html
/tmp/dir_b/e1d543811447e6672bb362899723ba84.html
/tmp/dir_b/index.html
/tmp/dir_b/report.css

There might be an added benefit if a unique html file is generated based on both pathname and contents. Since successive hurl runs append report data, but the source file html is regenerated each time, it will only represent the latest run as currently implemented, which will not be accurate if the input file changes between runs. Unique input file html files assed to the report at each run could capture their state at the time.

@fabricereix
Copy link
Collaborator

Hi @jleverenz,
Sorry, I had already worked on this one.
I've just pushed the MR (#534)

The HTML file were generated in the report-html directory only when the Hurl input files were specified relatively.

I had been mistaken by the API of join of PathBuf.
Path::new("/tmp").join("/tests/test1.hurl") returns /tests/test1.hurl and not /tmp/tests/test1.hurl

In the fix, the Hurl input path is always be converted to a relative path.

for example,
if the input file is /tmp/dir_a/test.hurl and the report-html directory is /tmp/dir_b,
the HTML file will be written in /tmp/dir_b/tmp/dir_a/test.hurl
and referenced relatively in the index.html file as tmp/dir_a/test.hurl

The full directory hierarchy is reproduced, therefore, we should not need any hash in the output filename.

@jleverenz
Copy link
Contributor

np, took a look at the PR, left a comment.

Also, there are some corner cases that I think might need consideration:

  • Input files of /tmp/test.hurl and ./tmp/test.hurl result in a collision in the report directory file location.
  • Input file of ../test.hurl results in a the file being created outside of the specified report-html directory.

@fabricereix
Copy link
Collaborator

For me, it's probably ok/acceptable to have collision for /tmp/test.hurl and ./tmp/test.hurl.
It does not seems very clean to call Hurl files with the same folder name both relatively and absolutely.

But I agree with your second point. A file should only be written below the report-html directory.
Therefore the generation of the output filename needs to be a more complicated than I had initially thought by stripping the leading /.

We could have an initial step to resolve the file in the filesystem.
and using the fullpath in the generated file, only if it is outside the current directory.

This is easier to explain with examples.
if the current directory is /tmp and the report-html directory is /tmp/dir_b.
We will have the different mappings (input file -> normalized full path -> generated HTML file)

/tmp/test.hurl              -> /tmp/test.hurl       -> /tmp/dir_b/test.hurl
test.hurl                   -> /tmp/test.hurl       -> /tmp/dir_b/test.hurl
../tmp/test.hurl            -> /tmp/test.hurl       -> /tmp/dir_b/test.hurl  
/dir_a/test.hurl            -> /dir_a/test.hurl     -> /tmp/dir_b/dir_a/test.hurl
../dir_a/test.hurl          -> /dir_a/test.hurl     -> /tmp/dir_b/dir_a/test.hurl
dir_c/../../dir_a/test.hurl -> /tmp/dir_a/test.hurl -> /tmp/dir_b/dir_a/test.hurl
dir_c/test.hurl             -> /tmp/dir_c/test.hurl -> /tmp/dir_b/dir_c/test.hurl
      

What do you think?

@jleverenz
Copy link
Contributor

I agree it's a corner case, but I'm having a hard time looking past it now that I know it's a possibility. Your proposal does look like it should address the original issue reported, though, which seems more urgent. Other issues in the discussion could be tracked separately. I think there are two that would remain after the path-normalization fix:

  1. The output filename collision "issue".
  2. The overwriting of output source files on successive hurl test runs, which could lead to inaccurate reporting for earlier runs if the input file changes.

@fabricereix
Copy link
Collaborator

Ok we accept the overwriting issue, it is inherent to to appending feature of the report.
This is only local, it can not happen in the CI, as everything is naturally recreated from scratch.

@fabricereix
Copy link
Collaborator

Hi @jleverenz,
I've just updated the PR.
I finally found that the previous algorithm was too complicated.

I'm now simply generate a relative output HTML filename from it full canonical path.
1/ the file is created by concatenating this relative path to the output_dir.
2/ the file is referenced in the index.html relatively.

Therefore, an input file will always generate the same output file, independently how it was called at the command-line.
This avoids the collision you mentioned.

For example
The input Hurl file /data/hurl/integration/tests_ok/hello.hurl:

  • will always generate an HTML file /tmp/report/data/hurl/integration/tests_ok/hello.hurl.html in the output_dir /tmp/report
  • will always be referenced in the index.html as data/hurl/integration/tests_ok/hello.hurl.html

The Hurl file could be called with the following filenames, they all generate the same HTML report.

/data/hurl/integration/tests_ok/hello.hurl
integration/tests_ok/hello.hurl
hello.hurl
../hello.hurl

@jleverenz
Copy link
Contributor

Sounds like a good simplification. Only downside I see is the inherently deep directory structure in the output_dir, but otherwise addresses the two issues you mention. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants