damianszczepanik / cucumber-reporting

HTML reports for Cucumber
GNU Lesser General Public License v2.1
548 stars 403 forks source link

Failure on number-looking buildNumber that is not an int #666

Closed melon3r closed 7 years ago

melon3r commented 7 years ago

My build numbers look like "20170919.3", which is a valid number (a float), which makes NumberUtils.isCreatable(buildNumber) return true in AbstractPage's buildGeneralParameters, but then it's treated as an int in Integer.parseInt(buildNumber), making it fail with an exception.

I'd make a PR to add a try&catch block around parseInt, but I don't know the code to tell if that'd break anything.

damianszczepanik commented 7 years ago

How it is possible that build is numbered as float and not number?

melon3r commented 7 years ago

It's actually a valid number, according to NumberUtils.isCreatable(buildNumber). But it's then treated as an Integer, which it isn't. See:

https://github.com/damianszczepanik/cucumber-reporting/blob/11c8c240c37ea73067fe0f7d1cc3ccbb938c01dd/src/main/java/net/masterthought/cucumber/generators/AbstractPage.java#L110-L118

The right thing in this case would be to not consider it a number, as it's really a date plus a counter... but at least it should not fail with an exception.

melon3r commented 7 years ago

I'd propose something like:

 // build number is not mandatory 
 String buildNumber = configuration.getBuildNumber(); 
 if (buildNumber != null) { 
     try { 
         context.put("build_previous_number", Integer.parseInt(buildNumber) - 1); 
     } catch (NumberFormatException e) { 
         LOG.info("Could not parse build number: {}.", configuration.getBuildNumber()); 
     } 
} 

or

 // build number is not mandatory 
 String buildNumber = configuration.getBuildNumber(); 
 if (buildNumber != null) {
     int n = NumberUtils.toInt(buildNumber);
     if (n > 0) {
         context.put("build_previous_number", n - 1); 
     } catch (NumberFormatException e) { 
         LOG.info("Could not parse build number: {}.", configuration.getBuildNumber()); 
     } 
} 
damianszczepanik commented 7 years ago

Build number comes from http://javadoc.jenkins-ci.org/hudson/model/Run.html#getNumber-- which is int value. How did you get the float then?

melon3r commented 7 years ago

I'm not using Jenkins. I'm setting the buildNumber through gradle, and it looks like [date].[counter].

This is my code:

def config = new XmlSlurper().parse(file('config.xml'))
if(config.projectName == null) {
    throw new InvalidUserDataException('Missing value in config.xml: \"projectName\"!')
}

if(config.buildNumber == null) {
    throw new InvalidUserDataException('Missing value in config.xml: \"buildNumber\"!')
}

boolean runWithJenkins = false
boolean parallelTesting = false

net.masterthought.cucumber.Configuration configuration = new net.masterthought.cucumber.Configuration(reportOutputDirectory, config.projectName.toString())

configuration.setParallelTesting(parallelTesting)
configuration.setRunWithJenkins(runWithJenkins)
configuration.setBuildNumber(config.buildNumber.toString())
ReportBuilder reportBuilder = new ReportBuilder(jsonReportFiles, configuration)
reportBuilder.generateReports()
damianszczepanik commented 7 years ago

This is interesting case. What then do you expect to have as the previous build? It could not be calculated automatically.

melon3r commented 7 years ago

Nothing at all really, as it's only used when runWithJenkins is true, and in my case it's false. Also, I store reports separately, and so navigating them wouldn't work.

https://github.com/damianszczepanik/cucumber-reporting/blob/d05232535cbf5638b211e9b6d5349d996fab6779/src/main/resources/templates/macros/page/navigation.vm#L7-L13

Maybe this code should check if it's running with Jenkins:

if (configuration.isRunWithJenkins()) {
    // build number is not mandatory
    String buildNumber = configuration.getBuildNumber();
    if (buildNumber != null) {
        if (NumberUtils.isCreatable(buildNumber)) {
            context.put("build_previous_number", Integer.parseInt(buildNumber) - 1);
        } else {
            LOG.info("Could not parse build number: {}.", configuration.getBuildNumber());
        }
    }
}
damianszczepanik commented 7 years ago

Now if you set setRunWithJenkins to false, you can pass any string you like. It will be displayed at the top table as the build number but won't be parsed for previous build.

melon3r commented 7 years ago

Thank you @damianszczepanik :) When can I expect a new release?

damianszczepanik commented 7 years ago

Very 4-6 weeks. But if you now set runWithJenkins to false it should work fine