Skip to content

Commit 3c82747

Browse files
mergify[bot]mergify[bot]
authored
Merge pull request #720 from retest/feature/retry-upload
Attempt re-upload if report upload to rehub fails
2 parents b15d05f + f5136f6 commit 3c82747

File tree

5 files changed

+80
-54
lines changed

5 files changed

+80
-54
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ Table of Contents
3939

4040
### Improvements
4141

42+
* Introduce property `de.retest.recheck.rehub.upload.attempts=3` to retry report uploads to rehub if they fail (e.g. due to unstable connections). This will reduce failing tests if the upload fails.
4243

4344
--------------------------------------------------------------------------------
4445

‎src/main/java/de/retest/recheck/RecheckProperties.java‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,57 +35,63 @@ static RecheckProperties getInstance() {
3535
* Various constants.
3636
*/
3737

38-
static final String PROPERTY_VALUE_SEPARATOR = ";";
39-
static final String ZIP_FOLDER_SEPARATOR = "/";
38+
String PROPERTY_VALUE_SEPARATOR = ";";
39+
String ZIP_FOLDER_SEPARATOR = "/";
4040

41-
static final String SCREENSHOT_FOLDER_NAME = "screenshot";
42-
static final String RECHECK_FOLDER_NAME = "recheck";
43-
static final String DEFAULT_XML_FILE_NAME = "retest.xml";
41+
String SCREENSHOT_FOLDER_NAME = "screenshot";
42+
String RECHECK_FOLDER_NAME = "recheck";
43+
String DEFAULT_XML_FILE_NAME = "retest.xml";
4444

45-
static final String RETEST_FOLDER_NAME = ".retest";
46-
static final String RETEST_PROPERTIES_FILE_NAME = "retest.properties";
45+
String RETEST_FOLDER_NAME = ".retest";
46+
String RETEST_PROPERTIES_FILE_NAME = "retest.properties";
4747

48-
static final String GOLDEN_MASTER_FILE_EXTENSION = ".recheck";
49-
static final String TEST_REPORT_FILE_EXTENSION = ".report";
50-
static final String AGGREGATED_TEST_REPORT_FILE_NAME = "tests" + TEST_REPORT_FILE_EXTENSION;
48+
String GOLDEN_MASTER_FILE_EXTENSION = ".recheck";
49+
String TEST_REPORT_FILE_EXTENSION = ".report";
50+
String AGGREGATED_TEST_REPORT_FILE_NAME = "tests" + TEST_REPORT_FILE_EXTENSION;
5151

5252
/*
5353
* Properties, their key constants and related functionality.
5454
*/
5555

56-
static final String IGNORE_ATTRIBUTES_PROPERTY_KEY = "de.retest.recheck.ignore.attributes";
56+
String IGNORE_ATTRIBUTES_PROPERTY_KEY = "de.retest.recheck.ignore.attributes";
5757

5858
@Key( IGNORE_ATTRIBUTES_PROPERTY_KEY )
5959
@DefaultValue( "" )
6060
@Separator( PROPERTY_VALUE_SEPARATOR )
6161
List<String> ignoreAttributes();
6262

63-
static final String ELEMENT_MATCH_THRESHOLD_PROPERTY_KEY = "de.retest.recheck.elementMatchThreshold";
63+
String ELEMENT_MATCH_THRESHOLD_PROPERTY_KEY = "de.retest.recheck.elementMatchThreshold";
6464

6565
@Key( ELEMENT_MATCH_THRESHOLD_PROPERTY_KEY )
6666
@DefaultValue( "0.3" )
6767
double elementMatchThreshold();
6868

69-
static final String ROOT_ELEMENT_MATCH_THRESHOLD_PROPERTY_KEY = "de.retest.recheck.rootElementMatchThreshold";
69+
String ROOT_ELEMENT_MATCH_THRESHOLD_PROPERTY_KEY = "de.retest.recheck.rootElementMatchThreshold";
7070

7171
@Key( ROOT_ELEMENT_MATCH_THRESHOLD_PROPERTY_KEY )
7272
@DefaultValue( "0.8" )
7373
double rootElementMatchThreshold();
7474

75-
static final String ROOT_ELEMENT_CONTAINED_CHILDREN_MATCH_THRESHOLD_PROPERTY_KEY =
75+
String ROOT_ELEMENT_CONTAINED_CHILDREN_MATCH_THRESHOLD_PROPERTY_KEY =
7676
"de.retest.recheck.rootElementContainedChildrenMatchThreshold";
7777

7878
@Key( ROOT_ELEMENT_CONTAINED_CHILDREN_MATCH_THRESHOLD_PROPERTY_KEY )
7979
@DefaultValue( "0.5" )
8080
double rootElementContainedChildrenMatchThreshold();
8181

82-
static final String REHUB_REPORT_UPLOAD_ENABLED_PROPERTY_KEY = "de.retest.recheck.rehub.reportUploadEnabled";
82+
String REHUB_REPORT_UPLOAD_ENABLED_PROPERTY_KEY = "de.retest.recheck.rehub.reportUploadEnabled";
8383

8484
@Key( REHUB_REPORT_UPLOAD_ENABLED_PROPERTY_KEY )
8585
@DefaultValue( "false" )
8686
boolean rehubReportUploadEnabled();
8787

88-
static final String FILE_OUTPUT_FORMAT_PROPERTY_KEY = "de.retest.output.Format";
88+
String REHUB_REPORT_UPLOAD_ATTEMPTS = "de.retest.recheck.rehub.upload.attempts";
89+
90+
@Key( REHUB_REPORT_UPLOAD_ATTEMPTS )
91+
@DefaultValue( "3" )
92+
int rehubReportUploadAttempts();
93+
94+
String FILE_OUTPUT_FORMAT_PROPERTY_KEY = "de.retest.output.Format";
8995

9096
@Key( FILE_OUTPUT_FORMAT_PROPERTY_KEY )
9197
FileOutputFormat fileOutputFormat();

‎src/main/java/de/retest/recheck/persistence/CloudPersistence.java‎

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import de.retest.recheck.report.TestReport;
1919
import kong.unirest.HttpResponse;
2020
import kong.unirest.Unirest;
21+
import kong.unirest.UnirestException;
2122
import lombok.extern.slf4j.Slf4j;
2223

2324
@Slf4j
@@ -34,15 +35,18 @@ public void save( final URI identifier, final T element ) throws IOException {
3435
kryoPersistence.save( identifier, element );
3536

3637
if ( isAggregatedReport( identifier ) && element instanceof TestReport ) {
38+
final TestReport report = (TestReport) element;
3739
try {
38-
saveToCloud( identifier, (TestReport) element );
39-
} catch ( final Exception e ) {
40-
if ( ((TestReport) element).containsChanges() ) {
41-
log.error( "The upload of the test report failed. The test report contains changes." );
42-
throw e;
43-
} else {
40+
saveToCloud( report, Files.readAllBytes( Paths.get( identifier ) ) );
41+
} catch ( final IOException e ) {
42+
if ( !report.containsChanges() ) {
4443
log.warn(
45-
"The upload of the test report failed. The test report does not contain any changes (excluding metadata differences)." );
44+
"Could not read report '{}' for upload. Ignoring exception because the report does not have any differences.",
45+
identifier, e );
46+
} else {
47+
log.error( "Could not read report '{}' for upload. Rethrowing because report has differences.",
48+
identifier, e );
49+
throw e;
4650
}
4751
}
4852
}
@@ -58,29 +62,46 @@ private List<String> getTestClasses( final TestReport report ) {
5862
.collect( Collectors.toList() );
5963
}
6064

61-
private void saveToCloud( final URI identifier, final TestReport report ) throws IOException {
65+
private void saveToCloud( final TestReport report, final byte[] data ) {
6266
final HttpResponse<String> uploadUrlResponse = getUploadUrl();
63-
64-
final ReportUploadMetadata metadata = ReportUploadMetadata.builder() //
65-
.location( identifier ) //
66-
.uploadUrl( uploadUrlResponse.getBody() ) //
67-
.testClasses( getTestClasses( report ) ) //
68-
.build();
69-
7067
if ( uploadUrlResponse.isSuccess() ) {
71-
uploadReport( metadata );
68+
final ReportUploadContainer metadata = ReportUploadContainer.builder() //
69+
.reportName( String.join( ", ", getTestClasses( report ) ) ) //
70+
.data( data ) //
71+
.uploadUrl( uploadUrlResponse.getBody() ) //
72+
.build();
73+
final boolean hasChanges = report.containsChanges();
74+
75+
final int maxAttempts = RecheckProperties.getInstance().rehubReportUploadAttempts();
76+
for ( int remainingAttempts = maxAttempts - 1; remainingAttempts >= 0; remainingAttempts-- ) {
77+
try {
78+
uploadReport( metadata );
79+
break; // Successful, abort retry
80+
} catch ( final UnirestException e ) {
81+
if ( !hasChanges ) {
82+
log.warn(
83+
"Failed to upload report. Ignoring exception because the report does not have any differences.",
84+
e );
85+
break;
86+
}
87+
if ( remainingAttempts == 0 ) {
88+
log.error(
89+
"Failed to upload report. Aborting, because maximum retries have been reached. If this happens often, consider increasing the property '{}={}'.",
90+
RecheckProperties.REHUB_REPORT_UPLOAD_ATTEMPTS, maxAttempts, e );
91+
throw e;
92+
} else {
93+
log.warn( "Failed to upload report. Retrying another {} times.", remainingAttempts, e );
94+
}
95+
}
96+
}
7297
}
7398
}
7499

75-
private void uploadReport( final ReportUploadMetadata metadata ) throws IOException {
76-
final String reportName = metadata.getTestClasses() //
77-
.stream() //
78-
.collect( Collectors.joining( ", " ) );
100+
private void uploadReport( final ReportUploadContainer metadata ) {
79101
final long start = System.currentTimeMillis();
80-
81102
final HttpResponse<?> uploadResponse = Unirest.put( metadata.getUploadUrl() ) //
82-
.header( "x-amz-meta-report-name", abbreviate( reportName, MAX_REPORT_NAME_LENGTH ) ) //
83-
.body( Files.readAllBytes( Paths.get( metadata.getLocation() ) ) ) //
103+
.header( "x-amz-meta-report-name", abbreviate( metadata.getReportName(), MAX_REPORT_NAME_LENGTH ) ) //
104+
.body( metadata.getData() ) //
84105
.asEmpty();
85106

86107
if ( uploadResponse.isSuccess() ) {

‎src/main/java/de/retest/recheck/persistence/ReportUploadContainer.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package de.retest.recheck.persistence;
2+
3+
import lombok.Builder;
4+
import lombok.Getter;
5+
6+
@Builder
7+
@Getter
8+
public class ReportUploadContainer {
9+
10+
private final String reportName;
11+
private final byte[] data;
12+
private final String uploadUrl;
13+
}

‎src/main/java/de/retest/recheck/persistence/ReportUploadMetadata.java‎

Lines changed: 0 additions & 15 deletions
This file was deleted.

0 commit comments

Comments
 (0)