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

Implemented feature hierarchy template #76

Merged

Conversation

cstrong
Copy link
Contributor

@cstrong cstrong commented May 25, 2017

New "hierarchy" report template (see #75 )
Fixed a few linter errors
Added some code comments
Made a few gratuitous changes in calculateDuration that made the code easier for me to understand, feel free to disagree

cstrong added 5 commits May 25, 2017 18:34
Make leaf features show up first before sub-suites in hierarchy report
Fix logic in hierarchy report to deal with subsuites that I broke when I generalized it to deal with features having no sub-structure.  oops.
@@ -82,26 +91,53 @@ var generateReport = function (options) {
sec > 0 ? formattedTimeStamp += sec + SECONDS : '';
ms > 0 ? formattedTimeStamp += ms + MILLI_SECONDS : '';

return formattedTimeStamp.trim().length === 0 ? '0s' : formattedTimeStamp;
return formattedTimeStamp.trim().length === 0 ? '<1ms' : formattedTimeStamp;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for displaying <1ms

lib/reporter.js Outdated
return suite;

});

suite.totalTime = calculateDuration(suite.totalTime);

suite.features = featureOutput;
setupSubSuiteTemplates(suite);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its breaking the existing functionality with other formatted reports when there is a hierarchy of feature files, with the templates bootstrap , foundation or simple.

To try - please move the happy.feaure and unhappy.features to respective directories, e.g. features/happy/happy.feature` and pls checkout the other templates HTML reports.

To run the tests npm run features

@@ -0,0 +1,45 @@
function drawChart(chartData) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep it DRY, Could we use the existing piechart.js from bootstrap directory?

@@ -0,0 +1,24 @@
$(document).ready(function() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep it DRY, Could we use the existing script.js from bootstrap directory?

since it is possible we have have a folder hierarchy but are using one of the "flat" reports.
Remove duplicate script.js and piechart.js files from hierarchy, instead refer to the sources from bootstrap.
@gkushang
Copy link
Owner

Nice work @cstrong ! It looks good overall. I've a suggestion in terms of view and displaying the report as a dashboard. Let me know what you think. I do have local changes that can be reviewed as a future improvements/PR.

Suggestion: Instead of wrapping all directories under All Features, render directly on the report (as shown in below screenshot). + the panels are aligned to pie-charts.

Reasons: One less click & can easily viewable on dashboard as overall results. Some of the users display the HTML report on a Dashboard (TV) which refreshes every few minutes. Having all features on single page would give them a nice overview.

screen shot 2017-05-30 at 3 40 52 pm

@cstrong
Copy link
Contributor Author

cstrong commented May 30, 2017

hmmm you make a convincing case! I agree the information along the top already gives you a summary, so the top "All Features" is a bit redundant. OK I am good with it! Let me know if you want to push your changes to this branch and merge it, or want to do it in a separate PR, or you want me to make the change.

I do have a concern about alignment, however. When I made the panels aligned with the pie charts, the scenarios got squished. Because we are adding potentially multiple levels of nesting we need more space, don't you think?

@gkushang
Copy link
Owner

I agree. I am afraid of nesting as well, but I believe it really depends on the users. The best results can be achieved for max 2 level of nesting, and I've hardly seen the nested directories beyond 2 levels. We must have to set some exceptions in README while using this template.

However, the scenarios panels are responsive and we would need minor alignment with the step-duration, otherwise it looks pretty. Below is the 2-level nesting

screen shot 2017-05-30 at 4 15 41 pm

@cstrong
Copy link
Contributor Author

cstrong commented May 30, 2017

works for me. The template only supports up to 3 levels of nesting anyway. I think that should be enough even for very large projects.

@gkushang gkushang merged commit 9565106 into gkushang:develop May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants