Skip to content

Performance testing base branch#166

Open
lameze wants to merge 4 commits intomainfrom
performanceTestingOficial
Open

Performance testing base branch#166
lameze wants to merge 4 commits intomainfrom
performanceTestingOficial

Conversation

@lameze
Copy link
Contributor

@lameze lameze commented Jan 30, 2026

First chuck of work to restore the performance job in our CI.

Probably there are some things that needs to be improved/fixed here and there but this is running in our CI without problems.

@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2026

Codecov Report

❌ Patch coverage is 0% with 145 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.07%. Comparing base (59e41cf) to head (de330f6).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
runner/main/jobtypes/performance/performance.sh 0.00% 126 Missing ⚠️
runner/main/modules/docker-jmeter/docker-jmeter.sh 0.00% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #166      +/-   ##
==========================================
- Coverage   63.15%   61.07%   -2.08%     
==========================================
  Files          51       56       +5     
  Lines        1699     2130     +431     
==========================================
+ Hits         1073     1301     +228     
- Misses        626      829     +203     
Flag Coverage Δ
behat_bisect_test 37.96% <0.00%> (-2.36%) ⬇️
behat_test 39.43% <0.00%> (-1.31%) ⬇️
phpunit_bisect_test 31.23% <0.00%> (-2.96%) ⬇️
phpunit_test 31.84% <0.00%> (-2.58%) ⬇️
postjobs_test 28.10% <0.00%> (-4.84%) ⬇️
runner_test 14.31% <0.00%> (-1.58%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

chmod -R 2777 "${SHAREDDIR}" || true

# Clone the performance data generator plugin inside the container.
docker exec "${WEBSERVER}" sh -c "git clone --depth 1 ${plugin_repo} ${dest}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we doing it this way? See the plugins module
https://github.com/moodlehq/moodle-ci-runner/blob/main/runner/main/modules/plugins/plugins.sh#L21

Essentially we shoudl be setting the PLUGINSTOINSTALL to something like:

PLUGINSTOINSTALL="${PLUGINSTOINSTALL};$plugin_repo|local/performancetool|main"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the runner code to use the PLUGINSTOINSTALL

echo "Running: ${initcmd[*]}"

plugin_repo="https://github.com/moodlehq/moodle-local_performancetool"
dest="/var/www/html/local/performancetool"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This won't work for Moodle 5.1 and above -- it isn't in the public directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed.

Comment on lines +120 to +122
local initcmd
performance_initcmd initcmd # By nameref.
echo "Running: ${initcmd[*]}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be just above the actual cal to the command, otherwise it can make debugging very confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have done a big clean up, it should be all good now.


# Ensure host shared directories exist and are writable so plugin can save files.
mkdir -p "${SHAREDDIR}/planfiles" "${SHAREDDIR}/output/logs" "${SHAREDDIR}/output/runs"
chmod -R 2777 "${SHAREDDIR}" || true
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's this about? Why do we need || true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

echo "Copying generated plan files from container to ${SHAREDDIR}/planfiles"

docker exec -u root "${WEBSERVER}" bash -lc "\
mkdir -p /shared/planfiles && \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't we already do this a few lines above? (the mkdir)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

--size="${SITESIZE}" \
--fixeddataset \
--bypasscheck \
--filesizelimit="1000"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the file size limit? Should it be in env vars?

@@ -0,0 +1,219 @@
import java.io.*;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this, and where is it used? Didn't we copy this into the performance tool system itself? I can't see anywhere that we consume this file.

@@ -0,0 +1,12 @@
import java.io.*;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto this file. Can't see it being used anywere right now.

# Include logs string.
includelogs=1
includelogsstr="-Jincludelogs=$includelogs"
samplerinitstr="-Jbeanshell.listener.init=recorderfunctions.bsf"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah - just found where we're using the recorder. Is this something that can / should be in the jmx files themselves?

$CFG->skiplangupgrade = false;

$CFG->wwwroot = 'http://host.name';
$CFG->wwwroot = getenv('MOODLE_WWWROOT') ?: 'http://host.name';
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this change the following error is displayed during the Jmeter run:
| http://host.name/course/view.php?id=4 Non HTTP response code: java.net.UnknownHostException | 3

@lameze lameze force-pushed the performanceTestingOficial branch from 2ba68c8 to 8daa21c Compare February 19, 2026 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants