Add fail option for regex extractor #6256
Conversation
src/components/src/main/java/org/apache/jmeter/extractor/RegexExtractor.java
Outdated
Show resolved
Hide resolved
| @@ -208,6 +214,8 @@ private JPanel makeParameterPanel() { | |||
| templateField = new JLabeledTextField(JMeterUtils.getResString("template_field")); //$NON-NLS-1$ | |||
| refNameField = new JLabeledTextField(JMeterUtils.getResString("ref_name_field")); //$NON-NLS-1$ | |||
| matchNumberField = new JLabeledTextField(JMeterUtils.getResString("match_num_field")); //$NON-NLS-1$ | |||
| failResultField = new JLabel(JMeterUtils.getResString("fail_if_not_matched_field")); | |||
| failResult = new JCheckBox(); | |||
There was a problem hiding this comment.
Could you use JEditableCheckBox here?
There was a problem hiding this comment.
emptyDefaultValue is also a JCheckBox(), so what would a JEditableCheckbox do better?
There was a problem hiding this comment.
I'm using JLabel + JCheckBox here because it's nicer to have the label in the same row as the others and the checkbox in the column of the input fields
There was a problem hiding this comment.
JEditableCheckbox would allow using ${..} expressions while JCheckBox, see #5944
There was a problem hiding this comment.
I see, although the JEditableCheckbox is a nice feature to paramaterize scripts, I don't think it's a good usecase on this specific checkbox. I don't see a reason to parameterize a specific regex extractor. The other checkboxes on the Regex Extractor aren't an editable checkbox as well.
I can give it a try tho, If it doesn't help, it doesn't hurt
| @@ -41,7 +41,7 @@ | |||
| * {@code @Isolated}. | |||
| */ | |||
| @Isolated | |||
| public abstract class JMeterTestCase { | |||
| public abstract class JMeterTestCase { | |||
There was a problem hiding this comment.
Please refrain from making changes that are not a part of the PR
There was a problem hiding this comment.
Sorry, somehow it was updated by a plugin I guess and didn't bothered fixing it. Will undo this
| @@ -57,7 +58,7 @@ public class RegexExtractor extends AbstractScopedTestElement implements PostPro | |||
| * These are passed to the setUseField() method | |||
| * | |||
| * Do not change these values! | |||
| */ | |||
| */ | |||
There was a problem hiding this comment.
Please refrain from making changes that are not a part of the PR
| log.debug("Input = '{}'", inputString); | ||
| return inputString; |
There was a problem hiding this comment.
Please refrain from making changes that are not a part of the PR
| @@ -472,7 +484,7 @@ private void initTemplate() { | |||
| PatternMatcher matcher = JMeterUtils.getMatcher(); | |||
| Pattern templatePattern = JMeterUtils.getPatternCache().getPattern("\\$(\\d+)\\$" // $NON-NLS-1$ | |||
| , Perl5Compiler.READ_ONLY_MASK | |||
| | Perl5Compiler.SINGLELINE_MASK); | |||
| | Perl5Compiler.SINGLELINE_MASK); | |||
There was a problem hiding this comment.
Please refrain from making changes that are not a part of the PR
| @@ -382,6 +382,7 @@ public void GUIComponents2(GuiComponentHolder componentHolder) throws Exception | |||
| IGNORED_PROPERTIES.add(LoopControllerSchema.INSTANCE.getContinueForever()); | |||
| IGNORED_PROPERTIES.add(RegexExtractorSchema.INSTANCE.getMatchTarget()); | |||
| IGNORED_PROPERTIES.add(RegexExtractorSchema.INSTANCE.getDefaultIsEmpty()); | |||
| IGNORED_PROPERTIES.add(RegexExtractorSchema.INSTANCE.getFailIfNotFound()); | |||
There was a problem hiding this comment.
Could you clarify why adding the property here? Ideally the list of "ignored properties" should be empty. I should add a clarification javadoc to IGNORED_PROPERTIES so it clarifies its purpose.
There was a problem hiding this comment.
I have no idea, if I don't add this line the build fails on the tests:
org.opentest4j.AssertionFailedError: GUI element class org.apache.jmeter.extractor.gui.RegexExtractorGui org.apache.jmeter.extractor.gui.RegexExtractorGui[${test_TestElement.name},0,0,0x0,invalid,layout=java.awt.BorderLayout,alignmentX=0.0,alignmentY=0.0,border=javax.swing.border.EmptyBorder@10142fa,flags=9,maximumSize=,minimumSize=,preferredSize=] be able to pass all the properties to a different TestElement ==> expected: <org.apache.jmeter.extractor.RegexExtractor::class {
props {
it[name] = "\${test_TestElement.name}"
it[guiClass] = "org.apache.jmeter.extractor.gui.RegexExtractorGui"
it[referenceName] = "\${test_RegexExtractor.refname}"
it[regularExpression] = "\${test_RegexExtractor.regex}"
it[template] = "\${test_RegexExtractor.template}"
it[default] = "\${test_RegexExtractor.default}"
it[matchNumber] = "\${test_RegexExtractor.match_number}"
it[failIfNotFound] = "\${test_RegexExtractor.fail_if_not_found}"
it[comments] = "\${test_TestPlan.comments}"
}
}
> but was: <org.apache.jmeter.extractor.RegexExtractor::class {
props {
it[name] = "\${test_TestElement.name}"
it[guiClass] = "org.apache.jmeter.extractor.gui.RegexExtractorGui"
it[comments] = "\${test_TestPlan.comments}"
it[referenceName] = "\${test_RegexExtractor.refname}"
it[regularExpression] = "\${test_RegexExtractor.regex}"
it[template] = "\${test_RegexExtractor.template}"
it[default] = "\${test_RegexExtractor.default}"
it[matchNumber] = "\${test_RegexExtractor.match_number}"
it[failIfNotFound] = false
}
}
| previousResult.setSuccessful(false); | ||
| AssertionResult res = new AssertionResult(getName()); | ||
| res.setFailure(true); | ||
| res.setFailureMessage("Pattern not found: " + getRegex()); |
There was a problem hiding this comment.
I suggest including the "source" that was checked against the regexp. In other words, something like `Pattern ... not found in ...<response body | ... >"
Not really, I'm not really into this. I've tried to make something out of an earlier existing (working) code for this feature as you've asked me to create a PR and would love this feature in JMeter, but I'm not a java developer. Can you help me out? |
Description
This change adds the option to automatically fail a sampler if a regular expression extraction can't find any matches.
Motivation and Context
This saves the need to add an assertion with a regex to be sure there are matches. This will either save doing the regex matching twice and/or improved the scripts by failing if the extraction fails rather fail later when there is no value
Based on this discussion. Thanks to @asfimport for initiating this and providing code
How Has This Been Tested?
I've tested this on HTTP Requests only with different types of matches (1: single extraction, 0: random and -1 to match multiple fields)
Screenshots (if appropriate):
Types of changes
Checklist: