MDBF-598 Remove checks for spider changes #180
Conversation
MDBF-535 enables conditional spider tests for protected branches in Buildbot when the commit has touched spider code. It is time to remove this condition as the trial shows these spider test runs have generally not been a problem for development.
|
@mariadb-YuchenPei I think it looks good. Only one question: this and MDBF-535 enables spider only for the branch protection builders. Should we also extend to all other builders? |
|
Hi Vlad,
On Wed 2023-09-27 02:40:13 -0700, Vlad Bogolin wrote:
@mariadb-YuchenPei I think it looks good. Only one question: this and
MDBF-535 enables spider only for the branch protection builders.
Should we also extend to all other builders?
I suppose it is ok to do so. I have a commit where I add spider to all
master-docker-nonstandard builders:
mariadb-YuchenPei@a9a00ac
I have a few questions regarding this commit:
0. Is it correct?
1. I thought fulltest already builds spider, but I am not sure how, so
I am adding dplugin_spider=yes there as well
2. Not sure if it makes sense to have spiders for all these builders.
Some builders like valgrind have a reputation for failure. Also, not
sure about bintar.
3. Should spider be added to other master.cfg's, like the ones under
master-llibvirt and master-nonlatent?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
Best,
Yuchen
|
|
I am not that sure about the nonstandard builders. As you said, they are notorious for failing in various circumstances, so I need to double check this with others. I was more referring to the f_quick_build builders. Only the branch protection use the protected_branches_mtr_additional_args, so that implies that all the non branch protection f_quick_build builders will run without having to --suite specified |
master-docker-nonstandard/master.cfg
Outdated
| f_asan_build.addStep(steps.MTR( | ||
| logfiles={"mysqld*": "/buildbot/mysql_logs.html", "syslog": "/var/log/syslog"}, | ||
| command=["sh", "-c", util.Interpolate('cd mysql-test && MTR_FEEDBACK_PLUGIN=1 ASAN_OPTIONS="abort_on_error=1" LSAN_OPTIONS="print_suppressions=0,suppressions=`pwd`/lsan.supp" perl mysql-test-run.pl --verbose-restart --force --retry=3 --max-save-core=1 --max-save-datadir=10 --max-test-fail=20 --mem --parallel=$(expr %(kw:jobs)s \* 2)', jobs=util.Property('jobs', default='$(getconf _NPROCESSORS_ONLN)'))], | ||
| command=["sh", "-c", util.Interpolate('cd mysql-test && MTR_FEEDBACK_PLUGIN=1 ASAN_OPTIONS="abort_on_error=1" LSAN_OPTIONS="print_suppressions=0,suppressions=`pwd`/lsan.supp" perl mysql-test-run.pl --verbose-restart --force --retry=3 --max-save-core=1 --max-save-datadir=10 --max-test-fail=20 --mem --parallel=$(expr %(kw:jobs)s \* 2) --suite=main,spider,spider/bg,spider/bugfix,spider/feature,spider/regression/e1121,spider/regression/e112122', jobs=util.Property('jobs', default='$(getconf _NPROCESSORS_ONLN)'))], |
There was a problem hiding this comment.
This looks good and enables spider. However, doesn't it have the potential to disable other tests? By not having --suite specified I assume that some default value is used, and in the new form now it only enables main and spider.
There was a problem hiding this comment.
Based on what I found online the default is
Comma separated list of suite names to run. The default, as of MariaDB 10.4.5, is:
"main-, archive-, binlog-, binlog_encryption-, csv-, compat/oracle-, encryption-, federated-, funcs_1-, funcs_2-, gcol-, handler-, heap-, innodb-, innodb_fts-, innodb_gis-, innodb_zip-, json-, maria-, mariabackup-, multi_source-, optimizer_unfixed_bugs-, parts-, perfschema-, plugins-, roles-, rpl-, sys_vars-, sql_sequence-, unit-, vcol-, versioning-,period-".
https://mariadb.com/kb/en/mariadb-test-run-pl-options/
So that would also turn off some tests
|
On Thu 2023-09-28 02:36:33 -0700, Vlad Bogolin wrote:
I am not that sure about the nonstandard builders. As you said, they
are notorious for failing in various circumstances, so I need to
double check this with others.
I was more referring to the f_quick_build builders. Only the branch
protection use the protected_branches_mtr_additional_args, so that
implies that all the non branch protection f_quick_build builders will
not have spider enabled.
It seems to me that there are two axes, there's
master-protected-branches, master-docker-nonstandard directory names,
etc. each having a master.cfg; then there's f_quick_build,
f_rpm_autobake, etc, defined in common_factories.py.
The protected_branches_mtr_additional_args that would run spider tests
unconditionally at commit ad11887 are defined and used in
master-protected-branches/master.cfg, by three builders amd64-fedora-38,
amd64-debian-10 and amd64-centos-7, all of which are f_quick_build. All
other builders there except tarball-docker are also f_quick_build.
Given your comment about nonstandard builders, I'm assuming that by "non
branch protection f_quick_build builders" you mean like all these other
builders in master-protected-branches/master.cfg except tarball-docker.
The problem is that they don't specify suites to begin with, so we need
to figure out a way to tell mtr to run the default suites plus spider
suites. I'm not sure how that can be done in a clean way.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
Best,
Yuchen
|
|
By nonstandard I refer to the builders from master-docker-nonstandard. I agree that there probably isn't a clean way to achieve this. Then I propose for now to enable it for the branch protection builders as you originally proposed in the first commit and further discuss for the other builders. |
mariadb-YuchenPei
force-pushed
the
main
branch
from
a9a00ac
to
ad11887
Compare
|
On Mon 2023-10-02 23:47:39 -0700, Vlad Bogolin wrote:
By nonstandard I refer to the builders from master-docker-nonstandard.
I agree that there probably isn't a clean way to achieve this. Then I
propose for now to enable it for the branch protection builders as you
originally proposed in the first commit and further discuss for the
other builders.
Sounds good. I've updated my PR branch accordingly so that it is one
commit on top of the current main.
Best,
Yuchen
|
|
On Mon 2023-10-02 23:47:39 -0700, Vlad Bogolin wrote:
By nonstandard I refer to the builders from master-docker-nonstandard.
It's the "non branch protection f_quick_build builders" that I find
confusing. What are these builders, are they the ones in
master-protected-branches/master.cfg that are not using
protected_branches_mtr_additional_args as mtr_additional_args, like
amd64-debian-11-debug-ps-embedded and amd64-ubuntu-2004-debug?
[... 9 lines elided]
Best,
Yuchen
|
|
Hi, Most of the builders use Related to the builders that you mentioned, if they don't define Best, |
|
On Tue 2023-10-03 23:05:37 -0700, Vlad Bogolin wrote:
Thanks for the reply @vladbogo.
I learned that once spider is built, spider tests will be part of the
default suites thanks to storage/spider/mysql-test/spider/suite.pm. So
spider tests actually run on more than those builders that use
protected_branches_mtr_additional_args, as my test of MDBF-535 where I
broke a spider test from two months ago shows[1]. And I am not sure we
need to enable more builders.
[1] https://buildbot.mariadb.org/#/grid?branch=bb-10.9-mdbf-535-test
In any case shall we go ahead and merge this PR?
Best,
Yuchen
|
|
Thanks. That's good to know. I merged the PR. It will not go live instantly though. |
MDBF-535 enables conditional spider tests for protected branches in Buildbot when the commit has touched spider code. It is time to remove this condition as the trial shows these spider test runs have generally not been a problem for development.