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

Rerun suite, test, url #412

Merged
merged 209 commits into from
Nov 8, 2018
Merged

Rerun suite, test, url #412

merged 209 commits into from
Nov 8, 2018

Conversation

plutasnyy
Copy link
Contributor

@plutasnyy plutasnyy commented Oct 23, 2018

Description

This PR provide you the functionality to rerun the whole suite - it runs in the same way like typical suite run, only test or url rerun - rerun part of suite work in a different way, after rerun, artifacts in your suite will be overwritten and aet doesn't create a new version of the suite.
The new way to run a test for page requires changes in the cleaner. The cleaner should remove overwritten artifacts- we don't want rubbish in our database. Functionality for the cleaner is in this PR

Motivation and Context

It would be a nice improvement for users

Screenshots (if appropriate):

44725131-e82f4200-aad4-11e8-9466-4d1994da8ca7
44725134-e9f90580-aad4-11e8-9b1a-59263d1b76ff
46950715-ec63ee80-d085-11e8-98db-9d7c609afb83

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I hereby agree to the terms of the AET Contributor License Agreement.

@@ -49,6 +50,9 @@ public void setStepResult(ComparatorStepResult stepResult) {
}

public List<Operation> getFilters() {
if(filters == null){

Choose a reason for hiding this comment

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

is it possible to pass the condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review ;)
Unfortunately, it is possible because the model in mongo has null when you saving empty array.
You can find more details here: discussion

@@ -52,8 +54,12 @@
private static final Type SUITE_TYPE = new TypeToken<Suite>() {
}.getType();

public void setCorrelationId(String correlationId) {

Choose a reason for hiding this comment

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

why do we have the setter so high in the class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 😄

@@ -191,7 +208,7 @@ public String toString() {
.toString();
}

public void setVersion(long version) {
public void setVersion(Long version) {

Choose a reason for hiding this comment

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

is it expected that version could be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes,
I remember that it is strongly connected with updating version of a suite. Suite version is null during the first run of and typical run but during rerun, the suite has a version, so I needed to allow on setting empty version and set the same state like in a typical run. I didn't see a better way to set the correct suite's version during the rerun.

Copy link
Contributor

@malaskowski malaskowski left a 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 SuiteReportFeatures docs and add some screenshots (e.g. those form the ticket description) and short note of the feature?
Also please update What's New with the short description or link to the Report Features Wiki Page.

break;
}
}
return Optional.ofNullable(urlToReturn);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just cosmetics and your implementation is perfectly fine. But you could also do this method body with streams:

Suggested change
return Optional.ofNullable(urlToReturn);
return urls.stream()
.filter(url -> url.getName().equals(urlName))
.findFirst();


public class SuiteExecutionProcessorStrategy extends ProcessorStrategy {

protected static final Logger logger = LoggerFactory
Copy link
Contributor

Choose a reason for hiding this comment

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

Name should be all upercase LOGGER a this is constant.


public class TestExecutionProcessorStrategy extends ProcessorStrategy {

protected static final Logger logger = LoggerFactory
Copy link
Contributor

Choose a reason for hiding this comment

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

Name should be all upercase LOGGER a this is constant.


public class UrlExecutionProcessorStrategy extends ProcessorStrategy {

protected static final Logger logger = LoggerFactory
Copy link
Contributor

Choose a reason for hiding this comment

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

Name should be all upercase LOGGER a this is constant.

@plutasnyy
Copy link
Contributor Author

Thank you for help with documentation and review

CHANGELOG.md Outdated
@@ -18,6 +18,7 @@ All notable changes to AET will be documented in this file.
- [PR-396](https://github.com/Cognifide/aet/pull/396) Added horizontal scrollbar for wide pages ([#393](https://github.com/Cognifide/aet/issues/393))
- [PR-403](https://github.com/Cognifide/aet/pull/403) Conditionally passed tests can be accepted ([#400](https://github.com/Cognifide/aet/issues/400))
- [PR-387](https://github.com/Cognifide/aet/pull/387) Set max allowed page screenshot height to 35k pixels.
- [PR-412](https://github.com/Cognifide/aet/pull/412) ([PR-336](https://github.com/Cognifide/aet/pull/336), [PR-337](https://github.com/Cognifide/aet/pull/337), [PR-395](https://github.com/Cognifide/aet/pull/395)) - Added rerun functionality for suite, test and url
Copy link
Contributor

Choose a reason for hiding this comment

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

@plutasnyy Please move this entry to "Unreleased" section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 😃

@malaskowski malaskowski added the QA Required Requires manual tests, possible regression or impact on existing features. label Nov 7, 2018
@Slasheruus
Copy link
Contributor

Tested (/)

@Slasheruus Slasheruus closed this Nov 8, 2018
@Slasheruus Slasheruus reopened this Nov 8, 2018
@tkaik tkaik merged commit 8f9698d into master Nov 8, 2018
@tkaik tkaik deleted the milestone/rerun-one-test branch November 8, 2018 14:01
@wblachowski wblachowski mentioned this pull request Oct 2, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Required Requires manual tests, possible regression or impact on existing features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants