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.

Sign up for GitHub

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 fail option for regex extractor #6256

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add fail option for regex extractor #6256

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0550942
Add fail option for regex extractor
Mar 22, 2024
67424b4
Add fail option for regex extractor
Mar 22, 2024
07cda84
Add unit tests
Nov 8, 2024
6d26e75
Merge branch 'master' into add-fail-regex-extractor
Nov 8, 2024
dffd95a
Undo unrelated changes
Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions src/components/src/main/java/org/apache/jmeter/extractor/RegexExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.regex.PatternSyntaxException;

import org.apache.commons.text.StringEscapeUtils;
import org.apache.jmeter.assertions.AssertionResult;
import org.apache.jmeter.processor.PostProcessor;
import org.apache.jmeter.samplers.SampleResult;
import org.apache.jmeter.testelement.AbstractScopedTestElement;
Expand Down Expand Up @@ -57,7 +58,7 @@ public class RegexExtractor extends AbstractScopedTestElement implements PostPro
* These are passed to the setUseField() method
*
* Do not change these values!
*/
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refrain from making changes that are not a part of the PR

public static final String USE_HDRS = "true"; // $NON-NLS-1$
public static final String USE_REQUEST_HDRS = "request_headers"; // $NON-NLS-1$
public static final String USE_BODY = "false"; // $NON-NLS-1$
Expand Down Expand Up @@ -126,6 +127,9 @@ private void extractWithOroRegex(SampleResult previousResult, JMeterVariables va
try {
pattern = JMeterUtils.getPatternCache().getPattern(regex, Perl5Compiler.READ_ONLY_MASK);
List<MatchResult> matches = processMatches(pattern, regex, previousResult, matchNumber, vars);
if(matches.isEmpty() && isFailIfNotFound()){
failResult(previousResult);
}
int prevCount = 0;
String prevString = vars.get(refName + REF_MATCH_NR);
if (prevString != null) {
Expand Down Expand Up @@ -178,6 +182,14 @@ private void extractWithOroRegex(SampleResult previousResult, JMeterVariables va
}
}

private void failResult(SampleResult previousResult){
previousResult.setSuccessful(false);
AssertionResult res = new AssertionResult(getName());
res.setFailure(true);
res.setFailureMessage("Pattern not found: " + getRegex());
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest including the "source" that was checked against the regexp. In other words, something like `Pattern ... not found in ...<response body | ... >"

previousResult.addAssertionResult(res);
}

private void extractWithJavaRegex(SampleResult previousResult, JMeterVariables vars, String refName, int matchNumber) {
String regex = getRegex();
java.util.regex.Pattern pattern = null;
Expand Down Expand Up @@ -244,8 +256,8 @@ private String getInputString(SampleResult result) {
: useBodyAsDocument() ? Document.getTextFromDocument(result.getResponseData())
: result.getResponseDataAsString() // Bug 36898
;
log.debug("Input = '{}'", inputString);
return inputString;
log.debug("Input = '{}'", inputString);
return inputString;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refrain from making changes that are not a part of the PR

}

private List<MatchResult> processMatches(Pattern pattern, String regex, SampleResult result, int matchNumber, JMeterVariables vars) {
Expand Down Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refrain from making changes that are not a part of the PR

if (log.isDebugEnabled()) {
log.debug("Pattern = '{}', template = '{}'", templatePattern.getPattern(), rawTemplate);
}
Expand Down Expand Up @@ -688,4 +700,13 @@ public boolean useMessage() {
public void setUseField(String actionCommand) {
set(getSchema().getMatchTarget(), actionCommand);
}

public void setFailIfNotFound(boolean isFailing) {
set(getSchema().getFailIfNotFound(), isFailing);
}

public boolean isFailIfNotFound() {
return get(getSchema().getFailIfNotFound());
}

}
18 changes: 18 additions & 0 deletions src/components/src/main/java/org/apache/jmeter/extractor/gui/RegexExtractorGui.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import javax.swing.ButtonGroup;
import javax.swing.JCheckBox;
import javax.swing.JComponent;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JRadioButton;

Expand All @@ -52,6 +53,7 @@ public class RegexExtractorGui extends AbstractPostProcessorGui {
private JLabeledTextField defaultField;
private JLabeledTextField matchNumberField;
private JLabeledTextField refNameField;
private JLabel failResultField;
private JRadioButton useBody;
private JRadioButton useUnescapedBody;
private JRadioButton useBodyAsDocument;
Expand All @@ -62,6 +64,7 @@ public class RegexExtractorGui extends AbstractPostProcessorGui {
private JRadioButton useMessage;
private ButtonGroup group;
private JCheckBox emptyDefaultValue;
private JCheckBox failResult;

public RegexExtractorGui() {
super();
Expand Down Expand Up @@ -93,6 +96,7 @@ public void configure(TestElement el) {
emptyDefaultValue.setSelected(re.isEmptyDefaultValue());
matchNumberField.setText(re.getMatchNumberAsString());
refNameField.setText(re.getRefName());
failResult.setSelected(re.isFailIfNotFound());
}
}

Expand Down Expand Up @@ -124,6 +128,7 @@ public void modifyTestElement(TestElement extractor) {
regex.setDefaultValue(defaultField.getText());
regex.setDefaultEmptyValue(emptyDefaultValue.isSelected());
regex.setMatchNumber(matchNumberField.getText());
regex.setFailIfNotFound(failResult.isSelected());
}
}

Expand All @@ -142,6 +147,7 @@ public void clearGui() {
emptyDefaultValue.setSelected(false);
refNameField.setText(""); //$NON-NLS-1$
matchNumberField.setText(""); //$NON-NLS-1$
failResult.setSelected(false);
}

private void init() { // WARNING: called from ctor so must not be overridden (i.e. must be private or final)
Expand Down Expand Up @@ -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();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you use JEditableCheckBox here?

Copy link
Author

Choose a reason for hiding this comment

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

emptyDefaultValue is also a JCheckBox(), so what would a JEditableCheckbox do better?

Copy link
Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

JEditableCheckbox would allow using ${..} expressions while JCheckBox, see #5944

Copy link
Author

Choose a reason for hiding this comment

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

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


JPanel panel = new JPanel(new GridBagLayout());
GridBagConstraints gbc = new GridBagConstraints();
Expand All @@ -220,6 +228,8 @@ private JPanel makeParameterPanel() {
resetContraints(gbc);
addField(panel, matchNumberField, gbc);
resetContraints(gbc);
addField(panel, failResultField, failResult, gbc);
resetContraints(gbc);
gbc.weighty = 1;

defaultField = new JLabeledTextField(JMeterUtils.getResString("default_value_field")); //$NON-NLS-1$
Expand Down Expand Up @@ -253,6 +263,14 @@ private static void addField(JPanel panel, JLabeledTextField field, GridBagConst
panel.add(item.get(1), gbc.clone());
}

private static void addField(JPanel panel, JLabel label, JCheckBox checkBox, GridBagConstraints gbc) {
panel.add(label, gbc.clone());
gbc.gridx++;
gbc.weightx = 1;
gbc.fill = GridBagConstraints.HORIZONTAL;
panel.add(checkBox, gbc.clone());
}

// Next line
private static void resetContraints(GridBagConstraints gbc) {
gbc.gridx = 0;
Expand Down
3 changes: 3 additions & 0 deletions src/components/src/main/kotlin/org/apache/jmeter/extractor/RegexExtractorSchema.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public abstract class RegexExtractorSchema : AbstractScopedTestElementSchema() {
public val defaultIsEmpty: BooleanPropertyDescriptor<RegexExtractorSchema>
by boolean("RegexExtractor.default_empty_value", default = false)

public val failIfNotFound: BooleanPropertyDescriptor<RegexExtractorSchema>
by boolean("RegexExtractor.fail_if_not_found", default = false)

public val template: StringPropertyDescriptor<RegexExtractorSchema>
by string("RegexExtractor.template")
}
1 change: 1 addition & 0 deletions src/core/src/main/resources/org/apache/jmeter/resources/messages.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,7 @@ export_transactions_menu=Export transactions for report
export_transactions_names_action=Export transactions for report
export_transactions_title=Export Transactions Result
expression_field=CSS Selector expression\:
fail_if_not_matched_field=Fail if not matched
field_name=Field name
file=File
file_already_in_use=That file is already in use
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/testFixtures/java/org/apache/jmeter/junit/JMeterTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
* {@code @Isolated}.
*/
@Isolated
public abstract class JMeterTestCase {
public abstract class JMeterTestCase {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please refrain from making changes that are not a part of the PR

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, somehow it was updated by a plugin I guess and didn't bothered fixing it. Will undo this

// Used by findTestFile
private static final String filePrefix;

Expand Down
1 change: 1 addition & 0 deletions src/dist-check/src/test/java/org/apache/jmeter/junit/JMeterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

@jgaalen jgaalen Mar 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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
        }
    }

// TODO: support expressions?
IGNORED_PROPERTIES.add(HTTPSamplerBaseSchema.INSTANCE.getIpSourceType());
IGNORED_PROPERTIES.add(HTTPSamplerBaseSchema.INSTANCE.getImplementation());
Expand Down