8000 Improved test setup by theseion · Pull Request #2363 · coreruleset/coreruleset · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Improved test setup #2363

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

Jump to bottom
Merged
merged 2 commits into from
Apr 1, 2022
Merged

Improved test setup #2363

merged 2 commits into from
Apr 1, 2022

Conversation

theseion
Copy link
Contributor

Improved CRS_Tests.py

Removed unused fixture parameters

Completely rewrote log checker to use the new marker feature instead of parsing log timestamps

Added a bit of magic to detect the correct log to scan. This should reduce the number of times you run tests with the wrong configuration

This PR relies on two other PR's: coreruleset/modsecurity-crs-docker#58, coreruleset/ftw#66.

@theseion theseion requested review from fgsch and fzipi January 30, 2022 17:07
@theseion theseion force-pushed the improved-test-setup branch from 1fdf76d to 0c36cf6 Compare January 30, 2022 17:10
@fzipi fzipi self-assigned this Jan 30, 2022
@fzipi
Copy link
Member
fzipi commented Jan 30, 2022

I see some util/regexp-assembly/data files changed. Is this part of this PR?

@theseion
Copy link
Contributor Author

I see some util/regexp-assembly/data files changed. Is this part of this PR?

No, that was an accident (I had been working on something else originally). I've removed those changes.

@theseion
Copy link
Contributor Author

Note: tests are failing because there is no version 1.3.0 of ftw yet. I need that PR to go through first.

@fzipi fzipi mentioned this pull request Feb 7, 2022
Closed
@dune73 dune73 mentioned this pull request Mar 7, 2022
Closed
@fzipi fzipi marked this pull request as ready for review March 29, 2022 12:58
@fzipi
Copy link
Member
fzipi commented Mar 29, 2022

ftw version 1.3.0 is out, this one can be tested now!

@theseion theseion force-pushed the improved-test-setup branch from 79e86b7 to 5cfbc2d Compare March 29, 2022 17:33
@fzipi
Copy link
Member
fzipi commented Apr 1, 2022

Are we ready to merge this one?

fzipi
fzipi reviewed Apr 1, 2022
View reviewed changes
Copy link
Member
@fzipi fzipi left a comment

Choose a reason for hiding this comment

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

How fast will this be running now? Comparable times to go-ftw? Do we need to keep tests split in that many parallel checks?

@fzipi
Copy link
Member
fzipi commented Apr 1, 2022

@theseion Yes, comparable: 1m 17s vs. 1m 20s.

I would say let's remove the tests matrix:

         tests: [REQUEST-911-METHOD-ENFORCEMENT,
                 REQUEST-913-SCANNER-DETECTION,
                 REQUEST-920-PROTOCOL-ENFORCEMENT,
                 REQUEST-921-PROTOCOL-ATTACK,
                 REQUEST-930-APPLICATION-ATTACK-LFI,
                 REQUEST-931-APPLICATION-ATTACK-RFI,
                 REQUEST-932-APPLICATION-ATTACK-RCE,
                 REQUEST-933-APPLICATION-ATTACK-PHP,
                 REQUEST-934-APPLICATION-ATTACK-GENERIC,
                 REQUEST-941-APPLICATION-ATTACK-XSS,
                 REQUEST-942-APPLICATION-ATTACK-SQLI,
                 REQUEST-943-APPLICATION-ATTACK-SESSION-FIXATION,
                 REQUEST-944-APPLICATION-ATTACK-JAVA,
                 RESPONSE-953-DATA-LEAKAGES-PHP]

And run it with --ruledir=./tests/regression/tests ?

@theseion
Copy link
Contributor Author
theseion commented Apr 1, 2022

Cool! I didn't have any numbers :) Yes, let's remove the matrix. I've always run all tests.

fzipi
fzipi approved these changes Apr 1, 2022
View reviewed changes
Copy link
Member
@fzipi fzipi left a comment

Choose a reason for hiding this comment

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

Now, this is progress! Good job on getting this to the finish line!

@theseion theseion merged commit af5ea8d into coreruleset:v3.4/dev Apr 1, 2022
@theseion
Copy link
Contributor Author
theseion commented Apr 1, 2022

Thanks for the help!

@theseion theseion deleted the improved-test-setup branch April 1, 2022 16:28
@fzipi fzipi mentioned this pull request Apr 4, 2022
Closed
studersi added a commit to studersi/coreruleset that referenced this pull request Apr 5, 2022
7C4A
@studersi
Revert "Improved test setup (coreruleset#2363)"
acce578 
This reverts commit af5ea8d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fzipi fzipi fzipi approved these changes

@fgsch fgsch Awaiting requested review from fgsch

Assignees

@fzipi fzipi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@theseion @fzipi
0