Externalize JMeter from TestGrid #729
Conversation
| private static final String JMETER_TEST_PATH = "jmeter"; | ||
| private static final String SHELL_SUFFIX = ".sh"; | ||
| private static final String PRE_STRING = "pre-scenario-steps"; | ||
| private static final String POST_STRING = "post-scenario-steps"; | ||
| private static final String SCENARIO_SCRIPT = "scenario.sh"; |
There was a problem hiding this comment.
Not required in this PR but shall we change this to read the scenario script name from testgrid.yaml in the scenario tests repo just like how we read deploy script and infra script?
There was a problem hiding this comment.
But the issue is there will be only one deploy script & infra script per test-plan and there will be multiple scenario scripts. So wouldn't it be hard to add the scenario script name in each test scenario rather than going to a convention?
There was a problem hiding this comment.
I guess we can use this name as the default. We can provide an option to over-ride this via testgrid.yaml if required. We should not dictate the script names of test writers. Our only requirement should be the .testgrid.yaml. :)
WDYT?
| } finally { | ||
| try { | ||
| if (inputStream != null) { | ||
| inputStream.close(); |
There was a problem hiding this comment.
Shall we use try-with-resources to reduce boiler-plate code?
| } | ||
| } | ||
|
|
||
| private TestCase addAttributes(StartElement startElement) { |
There was a problem hiding this comment.
Missing javadocs.
How about we change the method name from addAttributes to something more meaningful? Say, buildTestCaseFrom? The input parameter name can be something like jmeterSampleElement. wdyt?
| Attribute attribute = attributes.next(); | ||
| if (TEST_NAME_ATTRIBUTE.equals(attribute.getName().getLocalPart())) { | ||
| testCase.setName(attribute.getValue()); | ||
| } else if (TEST_STATUS_ATTRIBUTE.equals(attribute.getName().getLocalPart())) { |
There was a problem hiding this comment.
TEST_STATUS_ATTRIBUTE ('ec') does not seem to be available in all the jtl outputs. For example, this is the output I get for a typical http sampler with an assertion. May be we should check the 's' attribute.
<httpSample t="254" it="0" lt="249" ct="197" ts="1525345990802" s="false" lb="Get IS Home Page" rc="200" rm="OK" tn="Test Identity Server 1-1" dt="text" by="15779" sby="140" ng="1" na="1">
<assertionResult>
<name>Check whether IS Page is loaded</name>
<failure>true</failure>
<error>false</error>
<failureMessage>Test failed: headers expected to contain /WSO2 Carbon Server1/</failureMessage>
</assertionResult>
<responseData class="java.lang.String">
...
There was a problem hiding this comment.
We could set the TestCase as successful if the "assertionResult" node does not contain any error or failure message. But the attribute "ec" was there in every test case in identity-integration tests.
There was a problem hiding this comment.
OK, if you think assertionResult node is a better estimate, then we should use that.
May be the 'ec' attribute was there for identity jmeter scripts because of some convention they follow. It's not fool-proof as I've seen.
| * @throws JMeterResultParserException {@link JMeterResultParserException} when an error occurs while obtaining the | ||
| * instance of the parser | ||
| */ | ||
| public static Optional<JMeterResultParser> getParser(TestScenario testScenario, String testLocation) throws |
There was a problem hiding this comment.
getResultParser?
Seems this is not a getter since we are doing the actual parsing here.
There was a problem hiding this comment.
This is not parsing the entire content of jtl. But this is required to obtain the correct parser for parsing the JTL file. This is merely based on the start element of the JTL file.
| } finally { | ||
| if (inputStream != null) { | ||
| try { | ||
| inputStream.close(); |
| @@ -1,6 +0,0 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <jmeterTestPlan version="1.2" properties="3.2" jmeter="3.3 r1808647"> | |||
There was a problem hiding this comment.
I think we should still have jmeter based unit tests within testgrid after externalizing the jmeter as well.
Test phase is a core part of testgrid. We need some kind of tool to verify that phase. Jmeter is the ideal candidate for that.
Our previous impl downloaded jmeter dependency via maven. In the new impl, we are invoking jmeter via script. So, for that, we have to download extract it manually. This can be done via the TestNG @BeforeSuite step.
Shall we do it here or create a separate ticket to track this?
There was a problem hiding this comment.
+1. New issue [1] created for it.
[1]. #733
|
|
||
| </head> | ||
| <!-- ===================================== END HEADER ===================================== --> | ||
| <body><a id="top-of-page& |
There was a problem hiding this comment.
The JTL is 4000 lines it seems.. :) I don't think we need one this big for unit tests.
There was a problem hiding this comment.
New issue [1] created.
[1]. #734
Purpose
Resolves #701
Approach
JMeter has to be installed locally and JMETER_HOME env var should be added to the environment. TestGrid will invoke a scenario.sh script inside each scenario which in-turns use installed JMeter and run jmx files.
Security checks