-
Notifications
You must be signed in to change notification settings - Fork 49
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
Cleaner will remove artifacts without reference in suites e.g after rerun one test #336
Cleaner will remove artifacts without reference in suites e.g after rerun one test #336
Conversation
…ner-removes-artifacts-after-rerun
…ner-removes-artifacts-after-rerun
…lank lines, changed variables names in RemoveArtifactsTests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please also update wiki docs and changelog?
* @param dbKey - key with project and company name | ||
* @return Set of all artifacts id contained in database as String set or empty set | ||
*/ | ||
Set<String> getArtifactsId(DBKey dbKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getArtifactsIds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
} | ||
|
||
public int getSuitesToAggregate() { | ||
return suitesToAggregate; | ||
} | ||
|
||
public int addSuitesToAggregate(int suitesToAggregate) { | ||
return this.suitesToAggregate + suitesToAggregate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't be here +=
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -32,8 +32,8 @@ public SuiteMessageBody(Suite data, DBKey dbKey, boolean toRemove) { | |||
this.toRemove = toRemove; | |||
} | |||
|
|||
public boolean shouldBeRemoved() { | |||
return toRemove; | |||
public boolean shouldBeKeeped() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldBeKept()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody)); | ||
} | ||
|
||
private void setArtifactsIdToKeep(Set<String> artifactsIdToRemove) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is to keep or remove :) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be af28aa4 🎃
} | ||
|
||
@Test | ||
public void check_ifRemoveArtifactsWasCalled_expectFalse() throws Exception { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are testing dryRun
here, please add it to the name of this test.
ReferencedArtifactsMessageBody messageBody = (ReferencedArtifactsMessageBody) exchange.getIn() | ||
.getBody(); | ||
assertEquals(EMPTY_ARTIFACTS_ID_SET, | ||
removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be easier to just check if removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody)
is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm checking it there: https://github.com/Cognifide/aet/pull/336/files#diff-4b628417ba96f36a0a4eec9f4817ba27R144. Here I wanted to check substraction ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant:
assertTrue(removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody).isEmpty())
:)
@@ -57,4 +70,11 @@ public void process(Exchange exchange) throws Exception { | |||
artifactsToRemove.size(), messageBody.getDbKey(), messageBody.getData()); | |||
} | |||
} | |||
|
|||
public static Set<String> getArtifactsIdsToRemove(ArtifactsDAO artifactsDAO, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't have to be public
.
ReferencedArtifactsMessageBody messageBody = (ReferencedArtifactsMessageBody) exchange.getIn() | ||
.getBody(); | ||
assertEquals(EMPTY_ARTIFACTS_ID_SET, | ||
removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant:
assertTrue(removeArtifactsProcessor.getArtifactsIdsToRemove(artifactDAO, messageBody).isEmpty())
:)
…oves-artifacts-after-rerun
…oves-artifacts-after-rerun
…oves-artifacts-after-rerun
Cleaner has ability to remove orphan's artifacts
Description
Before, all suite's versions for each suites in project was aggregated and after that, artifacts from suites marked as 'toRemove' was removed. Now, all suites for each project are aggregating and we removing set of all suites in projects without suites marked as 'toKeep'
Motivation and Context
It change is required by new feature - rerun one test
Screenshots (if appropriate):
See new diagram in files changed: diagram
Types of changes
Checklist:
I hereby agree to the terms of the AET Contributor License Agreement.