Skip to content

Conversation

@LennDG
Copy link

@LennDG LennDG commented Nov 27, 2017

I added 2 .tmpl files to give the possibility to add custom metadata to features. This will override all regular metadata. The PR does not create any breaking changes to already existing code that uses the project.

One bug that I do not know how to fix is that when many variables are added, the list of Metadata variables in a Feature overview will start continuing outside of the Metadata overview box.

I added the documentation on how this feature is used and corrected some typos and errors in the already existing documentation.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 97.756% when pulling b21cbdd on LennDG:master into 1be6145 on wswebcreation:master.

@LennDG LennDG mentioned this pull request Nov 29, 2017
@wswebcreation wswebcreation self-assigned this Dec 2, 2017
Copy link
Collaborator

@wswebcreation wswebcreation left a comment

Choose a reason for hiding this comment

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

Hi @LennDG

Tnx for this PR, this one is smaller and better to check ;-).

If I now generate the report with the old testdata I get this in the overview

image

And this in the scenario template

image

Looks like the code isn't doing what it should do. Please make your changes be backwards compatible otherwise I can't approve the PR.

Please also look at the rest of the comments

const featuresOverviewIndex = path.resolve(reportPath, INDEX_HTML);

// Assign the correct overview template
var featuresOverviewTemplate;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please write this as and try to avoid the usage of var

FEATURE_METADATA_OVERVIEW_TEMPLATE = suite.customMetadata ? FEATURE_CUSTOM_METADATA_OVERVIEW_TEMPLATE : FEATURE_METADATA_OVERVIEW_TEMPLATE;

function _createFeatureIndexPages(suite) {

// Set custom metadata overview for the feature
var featuresMetadataOverview;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please write this as below and try to avoid the usage of var

FEATURE_METADATA_OVERVIEW_TEMPLATE = suite.customMetadata ? FEATURE_CUSTOM_METADATA_OVERVIEW_TEMPLATE : FEATURE_METADATA_OVERVIEW_TEMPLATE;

const FEATURE_FOLDER = 'features';
const FEATURES_OVERVIEW_INDEX_TEMPLATE = 'features-overview.index.tmpl';
const CUSTOMDATA_TEMPLATE = 'components/custom-data.tmpl';
const FEATURES_OVERVIEW_TEMPLATE = 'components/features-overview.tmpl';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make a let of this

const FEATURES_OVERVIEW_INDEX_TEMPLATE = 'features-overview.index.tmpl';
const CUSTOMDATA_TEMPLATE = 'components/custom-data.tmpl';
const FEATURES_OVERVIEW_TEMPLATE = 'components/features-overview.tmpl';
const FEATURES_OVERVIEW_CUSTOM_METADATA_TEMPLATE = 'components/features-overview-custom-metadata.tmpl'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your missing a ; here

const FEATURES_OVERVIEW_CHART_TEMPLATE = 'components/features-overview.chart.tmpl';
const SCENARIOS_OVERVIEW_CHART_TEMPLATE = 'components/scenarios-overview.chart.tmpl';
const FEATURE_OVERVIEW_INDEX_TEMPLATE = 'feature-overview.index.tmpl';
const FEATURE_METADATA_OVERVIEW_TEMPLATE = 'components/feature-metadata-overview.tmpl';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make a let of this

_.template(_readTemplateFile(FEATURES_OVERVIEW_INDEX_TEMPLATE))({
suite: suite,
featuresOverview: _.template(_readTemplateFile(FEATURES_OVERVIEW_TEMPLATE))({
featuresOverview: _.template(_readTemplateFile(featuresOverviewTemplate))({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep the old line here because we re-assigned the let

_: _
}),
featureMetadataOverview: _.template(_readTemplateFile(FEATURE_METADATA_OVERVIEW_TEMPLATE))({
featureMetadataOverview: _.template(_readTemplateFile(featuresMetadataOverview))({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep the old line here because we re-assigned the let

test/test.js Outdated
test.generate({
saveCollectedJSON: true,
jsonDir: './test/unit/data/json/',
jsonDir: './reports/',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please provide a new test file with new data and and a new location so we can generate the report for your specific changes

@LennDG
Copy link
Author

LennDG commented Dec 4, 2017

Alright I will incorporate your comments. Quite new to Javascript so I will take some time to figure out best practices.

…st and data for testing the custom metadata functionality
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 97.742% when pulling 8cceda9 on LennDG:master into 1be6145 on wswebcreation:master.

@LennDG
Copy link
Author

LennDG commented Dec 11, 2017

Sorry for the late update, I was sick last week.

The first issue is because the customMetadata flag was set to true for the old data. Since the old does not have metadata formed as [{name: "foo", value: "bar"}] it has some weird effects. In correct usage this should not happen. I added a new test and changed the variable assignments like you requested.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 97.742% when pulling 03f6376 on LennDG:master into 1be6145 on wswebcreation:master.

Copy link
Collaborator

@wswebcreation wswebcreation left a comment

Choose a reason for hiding this comment

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

I just got one question, if we look at the situation where you use the report (without browsers), do you also have an example of json-output you get. I prefer having the data more specific to prevent questions about the testdata in the future.

@@ -0,0 +1,100 @@
[
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi @LennDG

Tnx for the changes. I only think you missed something. If I now generate the files there is no custom-metadata in the report, there is also no custom metadata in the pushed JSON files

Copy link
Author

Choose a reason for hiding this comment

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

I overwrite the metadata of the JSON files and have not added a custom-metadata field. This is to use the existing code as much as possible without needing changes.

@@ -0,0 +1,21 @@
'use strict';
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I now look at this I think we can do it better like this in 1 file. I've also adjusted the paths so they will not overwrite each other. Hope you don't mind it.

'use strict';

const test = require('../lib/generate-report');

/**
 * Generate a report for browsers
 */
test.generate({
    saveCollectedJSON: true,
    jsonDir: './test/unit/data/json/',
    reportPath: './.tmp/browsers/',
    reportName: 'You can adjust this report name',
    customData: {
        title: 'Run info',
        data: [
            {label: 'Project', value: 'Custom project'},
            {label: 'Release', value: '1.2.3'},
            {label: 'Cycle', value: 'B11221.34321'},
            {label: 'Execution Start Time', value: 'Nov 19th 2017, 02:31 PM EST'},
            {label: 'Execution End Time', value: 'Nov 19th 2017, 02:56 PM EST'}
        ]
    }
});

/**
 * Generate a report with custom metadata
 */
test.generate({
    saveCollectedJSON: true,
    jsonDir: './test/unit/data/custom-metadata-json/',
    reportPath: './.tmp/custom-metadata/',
    reportName: 'You can adjust this report name',
    customMetadata: true,
    customData: {
        title: 'Run info',
        data: [
            {label: 'Project', value: 'Custom project'},
            {label: 'Release', value: '1.2.3'},
            {label: 'Cycle', value: 'B11221.34321'},
            {label: 'Execution Start Time', value: 'Nov 19th 2017, 02:31 PM EST'},
            {label: 'Execution End Time', value: 'Nov 19th 2017, 02:56 PM EST'}
        ]
    }
});

Copy link
Author

Choose a reason for hiding this comment

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

It's your project, I can't tell you what to do with filepaths :)

"name": "Ambiguous scenarios specified V1",
"tags": [],
"uri": "/Users/wswebcreation/deTesters/protractor-cucumber-typescript-boilerplate/e2e-tests/features/ambiguous.feature",
"metadata": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed

Copy link
Author

Choose a reason for hiding this comment

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

Do you mean that the file can be removed? I just overwrote the metadata field to re use it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No this is good 👍

@@ -0,0 +1,72 @@
[
{
"metadata": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed

@@ -0,0 +1,77 @@
[
{
"metadata": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed, I also think the App needs to be removed from the testdata so we have "clean" data

Copy link
Author

Choose a reason for hiding this comment

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

I cleaned out the App because it indeed didn't make sense and was confusing.

"name": "Pending scenarios specified V1",
"tags": [],
"uri": "/Users/wswebcreation/deTesters/protractor-cucumber-typescript-boilerplate/e2e-tests/features/pending.feature",
"metadata": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed

@@ -0,0 +1,278 @@
[
{
"metadata": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed

@@ -0,0 +1,90 @@
[
{
"metadata": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this can be removed

@LennDG
Copy link
Author

LennDG commented Dec 12, 2017

I will update the test data to reflect other use cases than browser versions.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 97.742% when pulling 8bfb31d on LennDG:master into 1be6145 on wswebcreation:master.

@LennDG
Copy link
Author

LennDG commented Dec 12, 2017

The pushed JSON files have their custom metadata under the metadata field. I decided against using a custom-metadata field to prevent users being confused by trying to use both simultaneously, which does not work.

@wswebcreation
Copy link
Collaborator

Hi @LennDG

It now looks great. Thanks for this feature! I'll merge it and make a new release

@wswebcreation wswebcreation merged commit b6f7a28 into WasiqB:master Dec 15, 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.

3 participants