Commit Graph

103 Commits

Author SHA1 Message Date
Xavier Morel
98bb01edfc [ADD] runbot_merge, tests: opentelemetry support
Mostly for tests: it can be really difficult to correlate issues as
there are 3 different processes involved (the test runner, the odoo
being tested, and dummy-central (as github)) and the intermixing of
logs between the test driver and the odoo being tested is
not *amazing*.

- `pytest-opentelemetry`'s `--export-traces` is the signal for running
  tests with tracing enabled, that way just having
  `pytest-opentelemetry` installed does not do anything untowards.
- Until chrisguidry/pytest-opentelemetry#34 is merged, should probably
  use the linked branch as the default / base mode of having a single
  trace for the entire test suite is not great when there are 460
  tests, especially as local clients (opentelemetry-desktop-viewer,
  venator) mostly work on traces and aren't very good at zooming on
  *spans* at least currently.
- Additionally, the conftest plugin adds hooks for propagating through
  the xmlrpc client (communications with odoo) and enables the
  requests instrumentor from the opentelemetry python contribs.
- The dummy `saas_worker` was moved into the filesystem, that makes it
  easier to review and update.
- A second such module was added for the otel instrumentation *of the
  odoo under test*, that instruments psycopg2, requests, wsgi, and the
  server side of xmlrpc.
- The git ops were instrumented directly in runbot_merge, as I've
  tried to keep `saas_tracing` relatively generic, in case it could be
  moved to community or used by internal eventually.

Some typing was added to the conftest hooks and fixtures, and they
were migrated from tmpdir (py.path) to tmp_path (pathlib) for
consistency, even though technically the `mkdir` of pathlib is an
annoying downgrade.

Fixes #835
2025-02-24 15:43:11 +01:00
Xavier Morel
e3b4d2bb40 [IMP] *: cleanup status contexts in tests
For historical reasons pretty much all tests used to use the contexts
legal/cla and ci/runbot. While there are a few tests where we need the
interactions of multiple contexts and that makes sense, on the vast
majority of tests that's just extra traffic and noise in the
test (from needing to send multiple statuses unnecessarily).

In fact on the average PR where everything passes by default we could
even remove the required statuses entirely...
2025-02-06 14:55:28 +01:00
Xavier Morel
6d5c539c77 [IMP] *: fork with main_branch_only
Should limit the risk of cases where the fork contains outdated
versions of the reference branches and we end up with odd outdated /
not up to date basis for branches & updates, which can lead to
confusing situations.
2025-02-06 12:41:03 +01:00
Xavier Morel
3b0e59a3cd [IMP] conftest: fstring-ify repo forking
Also use the fork's response for the fork's own URL instead of
composing it by hand.
2025-02-06 12:41:03 +01:00
Xavier Morel
e065206f79 [IMP] conftest: minor cleanups to repo creation
- Use f-strings where appropriate.
- Get the repo url from github instead of composing it by hand
- Resolve URI template for the contents endpoint instead of
  hand-building the URL. URI template implementation is janky AF.
2025-02-06 12:40:54 +01:00
Xavier Morel
88453aa462 [FIX] conftest: assert tunnel response is a proper URL
Since the tunnel value is now a path to a script to run (and get the
public facing address out of), it's pretty easy to give the path to
the wrong program e.g. `--tunnel ngrok` will run ngrok itself, which
is going to yield an error message and do nothing, and then break when
we try to use the nonsensical result (e.g. an empty string or an error
message) as a URL.

So assert that the result does parse as a URL-ish with at least a
scheme and netloc.
2025-02-04 08:16:31 +01:00
Xavier Morel
62dd3ee636 [ADD] tests: shitty support for context
The API doesn't correctly match the server-side `Environment`/`Model`
API, but neither did `with_user`, so probably fine for now, can
revisit this thing if I start needing it more than once in a blue
moon.
2025-01-28 15:53:04 +01:00
Xavier Morel
942570e60a [IMP] forwardport: outstanding FP reminders
- only remind weekly initially (not daily)
- root reminders on the forward port's creation, not the source's
  merge date
- cap reminder interval at 4 weeks (instead of doubling every time)
- track reminders on forward ports, don't share between siblings
- remove `forwardport_updated_before` from the testing system, it's
  now possible to just update `reminder_next` to a past date and see
  if it gets triggered (it should to nothing on sources, or on forward
  port in a state which does not warrant concern)

Fixes #801
2025-01-24 14:53:34 +01:00
Xavier Morel
594f5988b8 [IMP] conftest cleanup
- merge and sort imports
- convert a few legacy string formattings to f-strings
2024-12-16 09:11:19 +01:00
Xavier Morel
7eeeb18af4 [ADD] tests: support creating repos without webhooks
Most of the repos we create we want to hook into the odoo instance,
but in some rare cases we want repositories for just push/pull, in
that case webhooks configuration just creates log noise (as the events
get rejected on intake).
2024-12-16 09:11:19 +01:00
Xavier Morel
5210cfb59f [IMP] tests: better tree-reading support
- add support for reading commit entries
- add support for reading tree entries in non-recursive mode

Previously the helper would only return blob entries, which does not
allow testing submodule support.
2024-12-16 09:11:19 +01:00
Xavier Morel
2deeb914cc [REM] tests: capsys use
Seems like remnants of old debugging of the rate limiting, and can
create odd situations when it fires as it replaces `sys.stdout` with
an in-memory buffer, which doesn't have a `fileno()`, which breaks
`server._move`.
2024-12-16 09:11:19 +01:00
Xavier Morel
d6e1516f31 [IMP] runbot_merge: support arbitrary tunnel scripts
Rather than add individual tunnel methods to conftest, just allows
specifying a tunnel script and have that do whatever.

Protocol is uncomplicated: workers run the `$tunnel` with an arbitrary
port, script should create a tunnel to `localhost:$port`, print the
ingress of the tunnel to `STDOUT` with a terminating newline then
close `STDOUT`, and wait for `SIGINT` or `SIGTERM`, doing  whatever
cleanup they need when receiving either of those.

`ngrok` and `localtunnel` adapter scripts are provided out of the box,
though the ngrok one doesn't *really* work when using xdist without a
pre-spawned ngrok worker. Then again using xdist against github actual
is suicidal (because concurrency limits + rate limits) so likely
irrelevant.

Fixes #729
2024-12-09 16:02:28 +01:00
Xavier Morel
461db6a3e7 [ADD] runbot_merge: closing linked issues on merge
Requires parsing the commit messages as github plain doesn't collate
the info from there, but also the descriptions: apparently github only
adds those to the references if the PR targets the default
branch. That's not a limitation we want.

Meaning at the end of the day, the only thing we're calling graphql
for is explicit manual linking, which can't be tested without
extending the github API (and then would only be testable on DC), and
which I'm not sure anyone bothers with...

Limitations
-----------

- Links are retrieved on staging (so if one is added later it won't be
  taken in account).
- Only repository-local links are handled, not cross-repository.

Fixes #777
2024-12-09 16:02:28 +01:00
Xavier Morel
679d556c90 [FIX] project creation: handling of mergebot info
- don't *fail* in `_compute_identity`, it causes issues when the token
  is valid but doesn't have `user:email` access as the request is
  aborted and saving doesn't work
- make `github_name` and `github_email` required rather than ad-hoc
  requiring them in `_compute_identity` (which doesn't work correctly)
- force copy of `github_name` and `github_email`, with o2ms being
  !copy this means duplicating projects now works out of the box (or
  should...)

Currently errors in `_compute_identity` are reported via logging which
is not great as it's not UI visible, should probably get moved to
chatter eventually but that's not currently enabled on projects.

Fixes #990
2024-12-02 16:32:53 +01:00
Xavier Morel
bf4cc09aa4 [IMP] reporting if creating template DB fails
Before this, if creating the DB failed the next worker would find
themselves with an empty `template-` file, so they would take the path
to *create* a template database, which would fail when trying to
`mkdir` the `shared-` directory as the directory would already exist.

The problem with this is this module would likely immediately fail and
take down their worker, triggering a test suite failure for themselves
and likely hiding the *true* failure cause (not sure why the
originally failed worker isn't the first one to trigger a failure but
there you go).

By skipping the tests instead, we provide a lot more opportunity for
the "true" failure to be revealed.
2024-11-20 12:43:43 +01:00
Xavier Morel
acaf605472 [FIX] runbot_merge: implement __contains__ in test proxy model
`read_tracking_value` likely never worked correctly in both branches,
but worked in 15.0 because the failures to do anything useful happened
to end in the right case?
2024-11-20 12:34:02 +01:00
Xavier Morel
c7523c0429 [MERGE] runbot_merge, forwardport: latest updates
Got a bunch of updates since the initial attempt to migrate the
mergebot before the odoo days.
2024-11-19 12:18:59 +01:00
Xavier Morel
640392dc20 [FIX] significantly speed up local testing
The mergebot tests have always been pretty gentle on system load which
is nice, however it's just looking at the list of longest tests that I
realised / re-membered the hook wait duration is 10 seconds for the
benefit of github, which doesn't really matter locally. This means on
interaction / cron-heavy tests the test might only be using on the
order of 10% CPU or something, that is a waste of time.

TBF this is easily compensated by increasing the concurrency of the
test suite (e.g. from 16 to 32 when I switched machine, but it seems
as if not more sensible to lower the webhook wait delay to something
more reasonable. 1s seems to be a good fit here, on my new computer at
n=16 it leads to the test suite running in 15mn at 600% CPU (which is
pretty good on a 6/12 CPU as it loads the system heavily but doesn't
completely bog it down).

Reducing it to 0.5s, the test suite takes the same duration but CPU
load increases to 770%, and errors creep up, likely a mix of
concurrency issues in the DB and dummy-central sending webhooks too
slowly as we compete with it for CPU resources (could actually make
sense to restrict the number of threads tokio can use). Reducing
concurrency could make this work better, but I think at this point
we're in a pretty good state, it's even somewhat reasonable to run the
test suite sequentially (taking about 1h10 but being functionally
invisible in terms of load).
2024-10-18 10:19:28 +02:00
Xavier Morel
a45db1e089 [IMP] conftest: support for a more generic current_date 2024-10-07 08:04:25 +02:00
Xavier Morel
3a8b4684da [IMP] conftest: support for mapped(fn) 2024-10-07 08:03:36 +02:00
Xavier Morel
6a1b77b92c [ADD] runbot_merge: support for unstaged patches
Unstaged changes can be useful or necessary for some tasks
e.g. absolute emergency (where even faking the state of a staging is
not really desirable, if that's even possible anymore), or changes
which are so broad they're difficult to stage (e.g. t10s updates).

Add a new object which serves as a queue for patch to direct-apply,
with support for either text patches (udiff style out of git show or
format-patch) or commits to cherry-pick. In the former case, the part
of the show / format-patch before the diff itself is used for the
commit metadata (author, committer, dates, message) whereas for the
commit version the commit itself is reused as-is.

Applied patches are simply disabled for traceability.

Fixes #926
2024-10-03 12:06:00 +02:00
Xavier Morel
ef22529620 [ADD] support for recursive tree to the test GH proxy 2024-09-27 12:36:50 +02:00
Xavier Morel
26882c42aa [FIX] warning in test logs 2024-09-27 12:36:02 +02:00
Xavier Morel
3b8d392548 [FIX] pytest warnings
Backport of d7a78f89d0

- `choice` is not a proper type
- markers should be declared
2024-09-05 13:24:10 +02:00
Xavier Morel
d7a78f89d0 [FIX] conftest: pytest warning
Forgot to declare `defaultstatuses` when it was introduced in
f367a64481. pytest righteously warns
when markers are not declated, so do that (turns out running pytest
with `-We` can be useful, though it also requires `-W
ignore::DeprecationWarning::importlib._boostrap` because reportlab
does weird shit).
2024-08-09 12:51:00 +02:00
Xavier Morel
5dcdfb1138 [IMP] testing: avoid error on test failure / abort
It's not entirely clear but I assume if / when a test fails or is
aborted (via C-c) pytest kills the stdout somehow. Either way this
causes a bunch of `OSError` to be logged out as we try to write to
closed pipes.

If that occurs, assume the test is gone and just bail out of the
thread.
2024-08-09 10:05:42 +02:00
Xavier Morel
229ae45e11 [MERGE] *: triggerify mergebot and forwardport crons
A few crons (e.g.database maintenance) remain on timers, but most of
the work crons should be migrated to triggers.

The purpose of this change is mostly to reduce log spam, as crons get
triggered very frequently (most of them every minute) but with little
to do. Secondary purposes are getting experience with cron triggers,
and better sussing out dependencies between actions and crons /
queues.

Fixes #797
2024-08-05 08:54:07 +02:00
Xavier Morel
78cc8835ce [IMP] rubnbot_merge: avoid triggering every cron on every test
Since every cron runs on a fresh database, on the first `run_crons`
every single cron in the db will run even though almost none of them
is relevant.

Aside from the slight inefficiency, this creates unnecessary extra
garbage in the test logs.

By setting the `nextcall` of all crons to infinity in the template we
avoid this issue, only triggered crons (or the crons whose nextcall we
set ourselves) will trigger during calls.

This requires adjusting the branch cleanup cron slightly: it didn't
correctly handle the initial run (`lastcall` being false).
2024-08-05 08:03:56 +02:00
Xavier Morel
157657af49 [REM] *: default_crons fixture
With the trigger-ification pretty much complete the only cron that's
still routinely triggered explicitly is the cross-pr check, and it's
that in all modules, so there's no cause to keep an overridable
fixture.
2024-08-02 15:14:50 +02:00
Xavier Morel
dd17730f4c [IMP] *: crons tests running for better triggered compatibility
Mergebot / forwardport crons need to run in a specific ordering in
order to flow into one another correctly. The default ordering being
unspecified, it was not possible to use the normal cron
runner (instead of the external driver running crons in sequence one
at a time). This can be fixed by setting *sequences* on crons, as the
cron runner (`_process_jobs`) will use that order to acquire and run
crons.

Also override `_process_jobs` however: the built-in cron runner
fetches a static list of ready crons, then runs that.

This is fine for normal situation where the cron runner runs in a loop
anyway but it's any issue for the tests, as we expect that cron A can
trigger cron B, and we want cron B to run *right now* even if it
hadn't been triggered before cron A ran.

We can replace `_process_job` with a cut down version which does
that (cut down because we don't need most of the error handling /
resilience, there's no concurrent workers, there's no module being
installed, versions must match, ...). This allows e.g. the cron
propagating commit statuses to trigger the staging cron, and both will
run within the same `run_crons` session.

Something I didn't touch is that `_process_jobs` internally creates
completely new environments so there is no way to pass context into
the cron jobs anymore (whereas it works for `method_direct_trigger`),
this means the context values have to be shunted elsewhere for that
purpose which is gross. But even though I'm replacing `_process_jobs`,
this seems a bit too much of a change in cron execution semantics. So
left it out.

While at it tho, silence the spammy `py.warnings` stuff I can't do
much about.
2024-08-02 09:00:34 +02:00
Xavier Morel
32871c3896 [FIX] *: stray prints
Found a bunch of old leftover `print` calls which should not be in the
repo.
2024-08-02 08:59:45 +02:00
Xavier Morel
48d22b920a [FIX] mergebot coverage
- add context, in case that allows tracking overlap between tests
  eventually
- use `-m` to run odoo, requires backporting `odoo/__main__.py`
  but I'm not quite sure how I was running it previously, possibly
  via odoo-bin? It certainly doesn't seem to work out with a global
  `odoo` helper
2024-07-30 11:04:45 +02:00
Xavier Morel
5126fd8053 [IMP] core: ensure web.base.url is correctly set up
Finally went to fix the issue that web.base.url is not correctly set
up by the test suite and thus things like `PullRequest.url` don't
return the right URL when running tests.

The answer I re-discovered yet again is that one only has to log in as
an admin (`base.group_system`) while providing a `base_location` env
key.

This is done automatically when logging via the frontend, under the
assumption that this is the "correct" url, but when logging in via RPC
it has to be done manually as that's more internal and we might use
e.g. scripts on the same server to manipulate the instance.
2024-07-23 13:00:19 +02:00
Xavier Morel
a5c514ad6e [FIX] *bot: copy filestore (shared dir) when copying db
Previously we would copy *only* the database itself, which is mostly
not an issue, but when trying to debug *pages* using a browser, that
browser tries to load various assets, which are looked up in the
database, then in the filestore, where they are missing.

This is not an issue in and of itself, but it triggers ugly tracebacks
in the logs which is not convenient, and since
2ab06ca96b it fails the test even if the
test would otherwise work.
2024-07-23 13:00:19 +02:00
Xavier Morel
de32824a62 [IMP] *: move the page helper fixture to the shared conftest
Use it in `test_limit` instead of direct `requests` calls.
2024-06-26 15:17:09 +02:00
Xavier Morel
dc90a207d6 [ADD] runbot_merge: help command, and help on error
Fixes #898
2024-06-24 22:16:43 +02:00
Xavier Morel
f3a0a5c27c [FIX] runbot_merge: tracking message author on PullRequest events
d4fa1fd353 added tracking to changes
from *comments* (as well as a few hacks around authorship transfer),
however it missed two things:

First, it set the `change-author` during comments handling only, so
changes from the `PullRequest` hook e.g. open, synchronise, close,
edit, don't get attributed to their actual source, and instead just
fall back to uid(1). This is easy enough to fix as the `sender` is
always provided, that can be resolved to a partner which is then set
as the author of whatever changes happen.

Second, I actually missed one of the message hooks: there's both
`_message_log` and `_message_log_batch` and they don't call one
another, so both have to be overridden in order for tracking to be
consistent. In this case, specifically, the *creation* of a tracked
object goes through `_message_log_batch` (since that's a very generic
message and so works on every tracked object created during the
transaction... even though batch has a message per record anyway...)
while *updates* go through `_message_log`.

Fixes #895
2024-06-21 16:33:44 +02:00
Xavier Morel
2ab06ca96b [IMP] *: require explicitly specifying whether exceptions in logs are valid
Seems like a good idea to better keep track of the log of an Odoo used
to testing, and avoid silently ignoring logged errors.

- intercept odoo's stderr via a pipe, that way we can still write it
  back out and pytest is able to read & buffer it, pytest's capfd
  would not work correctly: it breaks output capturing (and printing
  on failure); and because of the way it hooks in it's unable to
  capture from subprocesses inheriting the standard stream, cf
  pytest-dev/pytest#4428
- update the env fixture to check that the odoo log doesn't have any
  exception on failure
- make that check conditional on the `expect_log_errors` marker, this
  way we can mark tests for which we expect errors to be logged, and
  assert that that does happen
2024-06-12 15:09:42 +02:00
Xavier Morel
fec3d39d19 [ADD] *: per-repository webhook secret
Currently webhook secrets are configured per *project* which is an
issue both because different repositories may have different
administrators and thus creates safety concerns, and because multiple
repositories can feed into different projects (e.g. on mergebot,
odoo-dev/odoo is both an ancillary repository to the main RD project,
and the main repository to the minor / legacy master-wowl
project). This means it can be necessary to have multiple projects
share the same secret as well, this then mandates the secret for more
repositories per (1).

This is a pain in the ass, so just detach secrets from projects and
link them *only* to repositories, it's cleaner and easier to manage
and set up progressively.

This requires a lot of changes to the tests, as they all need to
correctly configure the signaling.

For `runbot_merge` there was *some* setup sharing already via the
module-level `repo` fixtures`, those were merged into a conftest-level
fixture which could handle the signaling setup. A few tests which
unnecessarily set up repositories ad-hoc were also moved to the
fixture. But for most of the ad-hoc setup in `runbot_merge`, as well
as `forwardport` where it's all ad-hoc, events sources setup was just
appended as is. This should probably be cleaned up at one point, with
the various requirements collected and organised into a small set of
fixtures doing the job more uniformly.

Fixes #887
2024-06-06 11:07:57 +02:00
Xavier Morel
a6a37f8896 [FIX] runbot_merge: handling of staging cancellation
Move staging cancellation to the batch, remove its (complicated)
handling from the PRs.

This loses some precision in the cancellation message, but that could
likely be recovered (in part) by adding more precise checks &
diagnostic extractions in the compute.
2024-05-23 07:58:58 +02:00
Xavier Morel
75f29f9315 [IMP] conftest: avoid parsing json it if may not be
When asserting a status fails, it's possible that this is a 400 or
500-type error which does not yield JSON data at all (e.g. forgot to
start the proxy or dummy), in which case trying to dump the json data
is actively harmful as it triggers cascading failures. And it's not
like the output message is formatted, so a re-structured assertion
message is not helpful.
2024-05-16 10:37:50 +02:00
Xavier Morel
0e71f85802 [ADD] forwardport: some typing to conftest
It's not amazing, because cross-typing between the different conftests
and utility modules doesn't really work smoothly. So not entirely
convinced it's great.

While at it, inline a few `x.json()` local aliases when the jsons are
used in exclusive locations e.g. usually an assertion message on one
side and a result / process on the other.
2024-05-16 10:37:50 +02:00
Xavier Morel
7054c865d7 [FIX] *: ngrok tunnel for 3.x
ngrok 3 scrambled some of the tunnel configuration keys. Most notably,
it replaced the ill-named `bind_tls` by an explicit list of http
`schemes`. Although it *removed* `bind_tls` so the fix is necessary
for ngrok to work again (especially as ngrok2 is reaching EOL).

While at it, improve the tunnel setup somewhat:

- remove fixme which we're probably not going to fix after all
- if we spawn ngrok ourselves, keep the handle around so we can
- kill the process we spawned directly instead of looking it up
  somewhat awkwardly
2024-01-16 14:59:01 +01:00
Xavier Morel
2fb26f10fb [IMP] *: make dummy saas_worker module installable
And install it. And add a hook to trigger "ready crons" (including
triggered crons).

Rather convenient to install test helpers inside the SUT.
2023-08-31 08:58:25 +02:00
Xavier Morel
85a7890023 [CHG] runbot_merge: switch staging from github API to local
It has been a consideration for a while, but the pain of subtly
interacting with git via the ignominous CLI kept it back. Then ~~the
fire nation attacked~~ github got more and more tight-fisted (and in
some ways less reliable) with their API.

Staging pretty much just interacts with the git database, so it's both
a facultative github operator (it can just interact with git directly)
and a big consumer of API requests (because the git database endpoints
are very low level so it takes quite a bit of work to do anything
especially when high-level operations like rebase have to be
replicated by hand).

Furthermore, an issue has also been noticed which can be attributed to
using the github API (and that API's reliability getting worse): in
some cases github will fail to propagate a ref update / reset, so when
staging 2 PRs it's possible that the second one is merged on top of
the temporary branch of the first one, yielding a kinda broken commit
(in that it's a merge commit with a broken error message) instead of
the rebase / squash commit we expected.

As it turns out it's a very old issue but only happened very early so
was misattributed and not (sufficiently) guarded against:

- 41bd82244bb976bbd4d4be5e7bd792417c7dae6b (October 8th 2018) was
  spotted but thought to be a mergebot issue (might have been one of
  the opportunities where ref-checks were added though I can't find
  any reference to the commit in the runbot repo).
- 2be25052e147b151d1d8a5bc73cceb351586ce03 (October 15th, 2019) was
  missed (or ignored).
- 5a9fe7a7d05a9df7186072a7bffd60c6b428fd0e (July 31st, 2023) was
  spotted, but happened at a moment where everything kinda broke
  because of github rate-limiting ref updates, so the forensics were
  difficult and it was attributed to rate limiting issues.
- f10d03bf0f2e8f88f62a5d8356b84f714196130f (August 24th, 2023) broke
  the camel's back (and the head block): the logs were not too
  interspersed with other garbage and pretty clear that github ack'd a
  ref update, returned the correct oid when checking the ref, then
  returned the wrong oid when fetching it later on.

No Working Copy
===============

The working copy turns out to not be necessary, the plumbing commands
we *need* work just fine on a bare repository.

Working without a WC means we had to reimplement the high level
operations (rebase) by hand much as we'd done previously, *but* we
needed to do that anyway as git doesn't seem to provide any way to
retrieve the mapping when rebasing/cherrypicking, and cherrypicking by
commit doesn't work well as it can't really find the *merge base* it
needs.

Forward-porting can almost certainly be implemented similarly (with
some overhead), issue #803 has been opened to keep track of the idea.

No TMP
======

The `tmp.` branches are no more, the process of creating stagings is
based entirely around oids, if staging something fails we can just
abandon the oids (they'll be collected by the weekly GC), we only
need to update the staging branches at the very end of the process.

This simplifies things a fair bit.

For now we have stopped checking for visibility / backoff as we're
pushing via git, hopefully it is a more reliable reference than the
API.

Commmit Message Formatting
==========================

There's some unfortunate churn in the test, as the handling of
trailing newlines differs between github's APIs and git itself.

Fixes #247

PS: It might be a good idea to use pygit2 instead of the CLI
    eventually, the library is typed which is nice, and it avoids
    shelling out although that's really unlikely to be a major cost.
2023-08-25 15:06:04 +02:00
Xavier Morel
aefbdaf974 [IMP] *: client side sorted implementation
Allow sorting by a callback. Sort remains client-side.
2023-07-10 15:23:31 +02:00
Xavier Morel
d748d4b215 [IMP] *: create a single template db per module to test
Before this, when testing in parallel (using xdist) each worker would
create its own template database (per module, so 2) then would copy
the database for each test.

This is pretty inefficient as the init of a db is quite expensive in
CPU, and when increasing the number of workers (as the test suite is
rather IO bound) this would trigger a stampede as every worker would
try to create a template at the start of the test suite, leading to
extremely high loads and degraded host performances (e.g. 16 workers
would cause a load of 20 on a 4 cores 8 thread machine, which makes
its use difficult).

Instead we can have a lockfile at a known location of the filesystem,
the first worker to need a template for a module install locks it,
creates the templates, then writes the template's name to the
lockfile.

Every other worker can then lock the lockfile and read the name out,
using the db for duplication.

Note: needs to use `os.open` because the modes of `open` apparently
can't express "open at offset 0 for reading or create for writing",
`r+` refuses to create the file, `w+` still truncates, and `a+` is
undocumented and might not allow seeking back to the start on all
systems so better avoid it.

The implementation would be simplified by using `lockfile` but that's
an additional dependency plus it's deprecated. It recommends
`fasteners` but that seems to suck (not clear if storing stuff in the
lockfile is supported, it opens the lockfile in append mode). Here the
lockfiles are sufficient to do the entire thing.

Conveniently, this turns out to improve *both* walltime CPU time
compared to the original version, likely because while workers now
have to wait on whoever is creating the template they're not competing
for resources with it.
2023-07-10 15:23:31 +02:00
Xavier Morel
72281b0c63 [IMP] runbot_merge: allow running test suite without an explicit addons path 2023-06-22 14:38:10 +02:00
Xavier Morel
4a4252b4b9 [FIX] runbot_merge: holes in provisioning
- github logins are case-insensitive while the db field is CI the dict
  in which partners are stored for matching is not, And the caller may
  not preserve casing.

  Thus it's necessary to check the casefolded database values against
  casefolded parameters, rather than exactly.
- users may get disabled by mistake or when one leaves the project,
  they may also get switched from internal to portal, therefore it is
  necessary to re-enable and re-enroll them if they come back.
- while at it remove the user's email when they depart, as they likely
  use an organisational email which they don't have access to anymore

Side-note, but remove the limit on the number of users / partners
being created at once: because there are almost no modules in the
mergebot's instance, creating partner goes quite fast (compared to a
full instance), thus the limitation is almost certainly unnecessary
(creating ~300 users seems to take ~450ms).

Fixes ##776
2023-06-14 16:01:42 +02:00