picuscreative / testcafe-reporter-html

The HTML reporter for TestCafe
https://picuscreative.com/
22 stars 30 forks source link

Suggestion: have all code in src, not in lib folder. Get rid of output (lib/*.js) files #9

Closed timnederhoff closed 5 years ago

timnederhoff commented 5 years ago

Currently the code in the output file lib/index.js is more up to date than the source code src/index.js. E.g. sorting, test numbers and url to failed tests (errors) are implemented in te output but nog in the source code. It might be forgotten to commit the last changes in src/index.js (and update tests), and to remove lib/* from git? Now, when I checkout, install and build, I get another version than the one published on npmjs.com

timnederhoff commented 5 years ago

ah, I see that last commits were done in the lib/index.js as well. So can I assume that this is the source file? If so, the tests doen't make any sense as they are testing the results from the src/index.js file.

davidcunha commented 5 years ago

Hello @timnederhoff

I'll have a look into this issue. I saw that you that submitted a PR and then you close it.

davidcunha commented 5 years ago

@timnederhoff I spotted the issue you mentioned. The previous PR were done with changes that had impact only on lib/*.js. I'm currently updating the dependencies (which are pretty outdated) and fixing this situation. Thanks!

timnederhoff commented 5 years ago

cool, thanks! The PR was for a feature request for including screenshots (if enabled in TestCafe), but the tests failed, so I closed it so I can fix and append tests to have the pipeline working again.

davidcunha commented 5 years ago

@timnederhoff Thanks! I submitted a PR #10 with some majors changes. Probably it will be merged by tomorrow. Therefore the best option would be for you to review your feature request with the latest changes.

timnederhoff commented 5 years ago

Hi @davidcunha that was quick :) I saw that most of the functionality were 'rebased' to the src folder. However, some things I still miss in the source:

davidcunha commented 5 years ago

@timnederhoff Thanks. #10 was approved and merged into master (it's not released yet). Please submit a PR with the aferomentioned changes and screenshots feature that I will review and accept it! You can remove the testcount too.

lib contains the output of the build process with babel. If you want to modify the library the src folder is the place to go, if you want to publish it we'll still need the lib.

timnederhoff commented 5 years ago

Hi @davidcunha, thanks you for improving the code so much. I created a PR #11 for the screenshots.

timnederhoff commented 5 years ago

lib contains the output of the build process with babel. If you want to modify the library the src folder is the place to go, if you want to publish it we'll still need the lib.

That's right. As long a you run gulp build before publishing (and you always should IMHO), then you have always the latest generated version from code in the lib folder. There is no need to commit this back to your source repository (it's in the published package though). I created PR #12 to remove the lib folder and include it in the publishable package (.npmignore)

davidcunha commented 5 years ago

@timnederhoff Thanks for the PR! It's released in 1.4.0.