Improve automatic test suite's HTML reports

appleseedhq / appleseed

A modern open source rendering engine for animation and visual effects

https://appleseedhq.net/

MIT License

2.19k stars 329 forks source link

Improve automatic test suite's HTML reports #2903

Closed russkev closed 3 years ago

russkev commented 3 years ago

Addresses issue #2853 . Git hash and title now added to HTML report.

Addresses issue #2844. I replicated the the statistics that get displayed in the console in the HTML report. I originally intended the info to be at the top of the page but that needed a lot more work than putting it at the bottom, so I went with that instead.

russkev commented 3 years ago

Thanks for your feedback, @oktomus. I've attempted to address most of it in my latest commit.

russkev commented 3 years ago

Main changes are:

Results Class
- Contains methods for setting and accessing data needed for results statistics information
git_information method
- Refactored out from __write_header
Various name and style changes suggested by @oktomus

oktomus commented 3 years ago

Thanks for the changes @russkev :)

russkev commented 3 years ago

Thanks for the kind words @oktomus I'm happy to help. Also thank you for your detailed reviews. Having read Clean Code, I very much appreciate the importance of good naming so I'm thankful to have guidance in that area.

I just made a few tweaks. Mainly changing a few names and adding some comments.

Also, what do you think about also implementing a __report_success method in ReportWriter so that the report lists all tests that were run?

oktomus commented 3 years ago

Also, what do you think about also implementing a __report_success method in ReportWriter so that the report lists all tests that were run?

You mean including successful tests in the html report ? Right now, showing only the failed tests is very useful because you can easily see if something is wrong or not. For a complete report, there is always the txt report : http://testsuite.appleseedhq.net:8080/current-job/sandbox/tests/test%20scenes/testsuite_script_log.txt. But maybe you have a design in mind that allow to get all the tests, while still being able to quickly see the failures ? Although I'm not sure how seeing all tests could be used for

russkev commented 3 years ago

I have to admit, I haven't spent much time playing with the tool in practice so I'm not familiar with how it's used. I thought that it would be nice to have all the information from the tests in one nicely presented html file instead of split up over multiple documents.

The way I was thinking of doing it was to have the successful tests be just a single line and the failures would have the comparison images, maybe some red text, so that it would be easy to differentiate.

russkev commented 3 years ago

Okay, I've made those changes and merged the latest master. Let me know if there's anything else I can do.

oktomus commented 3 years ago

Thanks a lot @russkev !

@dictoon I think we can merge it now. What do you say ?

dictoon commented 3 years ago

I think we can merge it now. What do you say ?

Yes!

Thanks @russkev for your work, and @oktomus for the reviews.