-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add page item in as code v3 #29
base: v3
Are you sure you want to change the base?
Conversation
public static final String XML_TAG_NAME = "delay-action"; | ||
public static final String XML_DURATION_ATT = "duration"; | ||
public static final String XML_ISTHINKTIME_ATT = "isThinkTime"; | ||
public class ThinkTimeWriter extends AbstractDelayActionWriter { |
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.
This class is not part of the commit and is missing...
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.
Ah bad cut it is on other pr. So I will merge.
This class is in the other Pull request
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.
Some unit tests failed in my local env :
- RequestWriterTest.writeDynamicRequest() This test fails if we run the whole class because "WriterUtils.getGeneratedUids()" is called by other tests and the value is 6 instead of 2.
- In PageWriterTest.writePageTestClassic() and writePageTest() The TU fails because missing attribute slaProfileEnabled="false"
This is not a big deal but should be fixed
You're missing the documentation for this new step. you need to validate it with amandine. It should be referenced there i think https://www.neotys.com/documents/doc/modules/as-code/steps.html |
There is some test missing i think you need to confirm this with @claudecarpentier . I think you need to test from a yaml file your regex and it should correspond to an xml file. You need to launch Integration tests on neoload-root for no regression, many tests are located in neoload-root. |
Should be good if you create a branch with your neotys account from the repo neotys labs and not a fork, so our jenkins work and we can check sonar and tests |
@Value.Immutable | ||
@JsonSerialize(as = ImmutablePage.class) | ||
@JsonDeserialize(as = ImmutablePage.class) | ||
public interface Page extends Step, SlaElement { |
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.
Some tests specific for this class are missing.
You can refer to what we did previously for testing a new as-code element.
For this class: com.neotys.neoload.model.v3.project.scenario.CustomPolicyStep
We made/updated these classes:
- com.neotys.neoload.model.v3.project.scenario.CustomPolicyStepTest
- com.neotys.neoload.model.v3.validation.validator.CustomPolicyStepTest
- com.neotys.neoload.model.v3.validation.validator.ScenarioTest
- com.neotys.neoload.model.v3.binding.io.IOScenariosTest
@JsonSerialize(as = ImmutableRequest.class) | ||
@JsonDeserialize(as = ImmutableRequest.class) | ||
@Value.Immutable | ||
@Value.Style(validationMethod = ValidationMethod.NONE) | ||
public interface Request extends Step, SlaElement, AssertionsElement { | ||
String NAME = "name"; | ||
String DYNAMIC_RESOURCES = "dynamicResources"; |
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.
Some tests are missing for this parameter and his getter, you can refer to the tests already existing for others parameters and getter of this class.
@JsonSerialize(as = ImmutableRequest.class) | ||
@JsonDeserialize(as = ImmutableRequest.class) | ||
@Value.Immutable | ||
@Value.Style(validationMethod = ValidationMethod.NONE) | ||
public interface Request extends Step, SlaElement, AssertionsElement { | ||
String NAME = "name"; | ||
String DYNAMIC_RESOURCES = "dynamicResources"; |
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 need to update the doc with this new parameter
Add page action and dynamic page capabilities to v3 of as code.