Commit Graph

1973 Commits

Author SHA1 Message Date
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-Do
f8f435d468 [FIX] runbot: remove record iteration in onchange
A recent task introduced a record loop using self inside.

It doesn't respect an api multi classic loop but it isn't an issue
since onchange are always called on a single record.

Removing the loop on all onchange to clean it up.
2024-09-25 10:37:09 +02:00
Xavier Morel
98868b5200 [IMP] runbot_merge: add notifications on inactive branch interactions
Add warnings when trying to send comments / commands to PRs targeting
inactive branches.

This was missing leading to confusion, as one warning is clearly not
enough.

Fixes #941
2024-09-24 10:22:07 +02:00
Xavier Morel
e309e1a3a2 [FIX] runbot_merge: don't over-bump timeout
By updating the staging timeout every time we run `_compute_state` and
still have a `pending` status, we'd actually bump the timeout *on
every success CI* except for the last one. Which was never the
intention and can add an hour or two to the mergebot-side timeout.

Instead, add an `updated_at` attribute to statuses (name taken from
the webhook payload even though we don't use that), read it when we
see `pending` required statuses, and update the staging timeout based
on that if necessary.

That way as long as we don't get *new* pending statuses, the timeout
doesn't get updated.

Fixes #952
2024-09-20 12:17:17 +02:00
Xavier Morel
154e610bbc [IMP] *: modernize TestPRUpdate
- Switch to just `default` ci.
- Autouse fixture to init the master branch.
- Switch to `make_commits`.
- Merge `test_reopen_update` and `test_update_closed_revalidate` into
  `test_update_closed`: the former did basically nothing useful and
  the latter could easily be folded into `test_update_closed` by just
  validating the new commit.
2024-09-19 12:17:59 +02:00
odoo
0b5d7d0566 [IMP] add customer on runbot build tasks
Idea is to help runbot/QA team to monitor other peoples tasks
easier. This way we will have a responsible person for each task
that someone else has assigned.
The goal of pr is two-fold:
1) We need to be able to better monitor the progress of tasks that
are not our own.
2) It introduces another metric with which we can measure individual
progress, and this can become main measuring metric for future non-technical
employees whose main goal will be making sure that none of the tasks become
rotten(not have any visible progress for months).
2024-09-19 08:20:14 +02:00
Xavier Morel
c8a06601a7 [FIX] *: unstage on status going from success to failure
And unconditionally unstage when the HEAD of a PR is synchronised.

While a rebuild on a PR which was previously staged can be a false
positive (e.g. because it hit a non-derministic test the second time
around) it can also be legitimate e.g. auto-rebase of an old PR to
check it out. In that case the PR should be unstaged.

Furthermore, as soon as the PR gets rebuilt it goes back into
`approved` (because the status goes to pending and currently there's
no great way to suppress that in the rebuild case without also fucking
it up for the sync case). This would cause a sync on the PR to be
missed (in that it would not unstage the PR), which is broken. Fix
that by not checking the state of the PR beforehand, it seems to be an
unnecessary remnant of older code, and not really an optimisation (or
at least one likely not worth bothering with, especially as we then
proceed to perform a full PR validation anyway).

Fixes #950
2024-09-18 15:19:13 +02:00
Xavier Morel
a046cf2f7c [FIX] *: UX around fw=no
The UX around the split of limit and forward port policy (and
especially moving "don't forward port" to the policy) was not really
considered and caused confusion for both me and devs: after having
disabled forward porting, updating the limit would not restore it, but
there would be no indication of such an issue.

This caused odoo/enterprise#68916 to not be forward ported at merge
(despite looking for all the world like it should be), and while
updating the limit post-merge did force a forward-port that
inconsistency was just as jarring (also not helped by being unable to
create an fw batch via the backend UI because reasons, see
10ca096d86).

Fix this along most axis:

- Notify the user and reset the fw policy if the limit is updated
  while `fw=no`.
- Trigger a forward port if the fw policy is updated (from `no`) on a
  merged PR, currently only sources.
- Add check & warning to limit update process so it does *not* force a
  port (though maybe it should under the assumption that we're
  updating the limit anyway? We'll see).

Fixes #953
2024-09-17 11:31:20 +02:00
Xavier Morel
851656bec0 [IMP] runbot_merge: set status on skipchecks & use that
- rather than enumerate states, forward-porting should just check if
  the statuses are successful on a PR
- for the same consistency reasons explained in
  f97502e503, `skipchecks` should force
  the status of a PR to `success`: it very odd that a PR would be
  ready without being validated...
2024-09-16 12:49:23 +02:00
Xavier Morel
fe7cd8e1f0 [IMP] runbot_merge: name PR correctly on staging success
Logging PRs by id is unusual, unreadable, and inconvenient.
2024-09-16 12:48:42 +02:00
Xavier Morel
10ca096d86 [FIX] forwardport: cron create prototype
Apparently when I added these to trigger the corresponding cron the
lack of decorator caused them to not work at all, at least when
invoked from the interface, consequently I notably wasn't able to
force a forward port via creating one such task when trying to work
around #953
2024-09-16 12:46:44 +02:00
Christophe Monniez
3ae3f6dff3 [IMP] runbot: pull docker image only when needed
When a lot of Docker images are updated at the same time, all runbot
hosts will try to pull them at the same moment.

With this commit, only the images marked as `always_pull` will be pulled
and if one of them takes too much time to be pulled, we let the host
make another loop turn before continuing the images pull.

Finally, if a host uses the Docker registry, it will pull the remote
image when running a step, that way the image will be pulled
automatically if needed.
2024-09-13 11:57:07 +02:00
Xavier-Do
b60d54d4ea [FIX] runbot: fix image build failure 2024-09-12 13:08:00 +02:00
Xavier-Do
b96b32e8b9 [IMP] runbot: better docker logging
The docker operation are called often and cannot be logged each time.
If a docker operation is slow, we log it at the end but waiting for this
output we have no idea what the process is doing.

This pr proposed to go a lower level and get the stream from the docker
operation to be able to log earlier if we started a potentially slow
operation.
2024-09-11 13:16:13 +02:00
Xavier-Do
14bac37438 [IMP] runbot: small change of docker_build api
returns now a dict instead of a tuple for easier extension
2024-09-11 13:16:13 +02:00
Christophe Monniez
e4b04a42d1 [FIX] runbot: remove leftover tuple in markdown log 2024-09-09 08:07:42 +02:00
Xavier Morel
60188063f8 [FIX] *: ensure I don't get bollocked up again by tags
Today (or really a month ago) I learned: when giving git a symbolic
ref (e.g. a ref name), if it's ambiguous then

1. If `$GIT_DIR/$name` exists, that is what you mean (this is usually
   useful only for `HEAD`, `FETCH_HEAD`, `ORIG_HEAD`, `MERGE_HEAD`,
   `REBASE_HEAD`, `REVERT_HEAD`, `CHERRY_PICK_HEAD`, `BISECT_HEAD` and
   `AUTO_MERGE`)
2. otherwise, `refs/$name` if it exists
3. otherwise, `refs/tags/$name` if it exists
4. otherwise, `refs/heads/$name` if it exists
5. otherwise, `refs/remotes/$name` if it exists
6. otherwise, `refs/remotes/$name/HEAD` if it exists

This means if a tag and a branch have the same name and only the name
is provided (not the full ref), git will select the tag, which gets
very confusing for the mergebot as it now tries to rebase onto the tag
(which because that's not fun otherwise was not even on the branch of
the same name).

Fix by providing full refs to `rev-parse` when trying to retrieve the
head of the target branches. And as a defense in depth opportunity,
also exclude tags when fetching refs by spec: apparently fetching a
specific commit does not trigger the retrieval of tags, but any sort
of spec will see the tags come along for the ride even if the tags are
not in any way under the fetched ref e.g. `refs/heads/*` will very
much retrieve the tags despite them being located at `refs/tags/*`.

Fixes #922
2024-09-06 15:09:08 +02:00
Xavier Morel
d0723499a2 [IMP] runbot_merge: stage by first ready
This is an approximation under the assumption that stored computes
update the `write_date`, and that there's not much else that will be
computed on a batch.

Eventually it might be a good idea for this to be a proper field,
computed alongside the unblocking of the batch.

Fixes #932
2024-09-06 13:51:55 +02:00
Xavier Morel
146564a90a [FIX] runbot_merge: set staging_end on all terminations
Rather than only setting `staging_end` on status change, set it when
the staging gets deactivated. This way cancelling a staging (whether
explicitely or via a PR update) will also end it, so will a staging
timeout, etc..., rather than keep the counter running.

Fixes #931
2024-09-06 13:16:37 +02:00
Xavier Morel
64f9dcbc22 [FIX] *: unnecessary warning on r- of forward port
Reminding users that `r-` on a forward port only unreviews *that*
forwardport is useful, as `r+;r-` is not a no-op (all preceding
siblings are still reviewed).

However it is useless if all siblings are not approved or already
merged. So avoid sending the warning in that case.

Fixes #934
2024-09-06 13:04:13 +02:00
Xavier Morel
017b8d4bb0 [FIX] forwardport: don't post reminder on PRs waiting to be staged
The FW reminder is useful to remind people of the outstanding forward
ports they need to process, as taking too long can be an issue.

They are, however, not useful if the developer has already done
everything, the PR is ready and unblocked, but the mergebot has fallen
behind and has a hard time catching up. In that case there is nothing
for the developer to do, so pinging them is not productive, it's only
frustrating.

Fixes #930
2024-09-06 12:33:51 +02:00
Christophe Monniez
33452d27e7 [IMP] runbot: add the possibility to disable a test per version
When disabling tests on runbot by using the test_tags on a build error,
the tests are disabled on every tested version. This can be annoying
when a test only fails from one version up to another.

With this commit, a min and max version can be specified on a
build_error when the test_tags field is used.
If the min and max are not used, the test will be disabled cross
versions.
2024-09-06 11:09:40 +02:00
Xavier-Do
dff50b49c4 [IMP] runbot: Improve stats listing display
Sort trigger and group them by category, better selection of categories
to display on stats page
2024-09-06 09:57:45 +02:00
Christophe Monniez
bc5e7fe69c [IMP] runbot: bump chrome version to 126.0.6478.182 2024-09-05 15:34:31 +02:00
Xavier-Do
21d6c84b26 [FIX] runbot: fix markdown adding escape
A common error on runbot is to generate link containing a __init__.py
file

[/some/path/to/__init__.py](/some/path/to/__init__.py)

This would be rendered as

<a href="/some/path/to/<ins>init<ins>.py">/some/path/to/<ins>init<ins>.py</a>

Breaking the link, and the display of the name

By default markdown will not render links avoiding this issue, but it
will remain for the content of the a, needing to manage some kind of
escaping.

The way to escape markdown is to add a \ before any special character
This must be done upront before formating, adding the method
markdown_escape

Our implementation of markdown is not meant to meet the exact
specification of markdown but better suit our needs. One of the
requirements is to be able to use it to format message easily but adding
dynamic countent comming from the outside. One of the error than can
occur is also

'Some code `%s`' % code can also cause problem if code contains `

This issue could be solved using indented code block, but this would
need to complexify the generated string, have a dedicated method to
escape the code blocs, ...

Since we have the controll on the input, we can easily sanitize all
ynamic content to avoid such issues. For code block we introduce a way
to escape backtick (\`). It is non standard but will be easier to use.

Combine with that, the build._log method now allows to add args with the
values to format the string (similar to logging) but will escape
params by default. (cr.execute spirit)

name = '__init__.py'
url = 'path/to/__init__.py'
code = '# comment `for` something'
build._log('f', 'Some message [%s](%s) \n `%s`', name, url, code)

name, url and code will be escaped
2024-09-05 15:33:41 +02:00
Xavier-Do
2fd7f47e49 [IMP] runbot: make stats when build timeout
The stats are only kept when the build finishes, this makes sence to
avoid collecting stats of a manually killed build (or because of newer
build found), but the stats mays be interresting when the build timeouts.

Adding a manual collection of stats in this case.
2024-09-05 14:24:41 +02:00
Xavier-Do
bc8c2a4853 [IMP] runbot: faster fetch
Use 2 jobs to fetch multiple origin at the same time.
2024-09-05 14:20:06 +02:00
Xavier Morel
1d106f552d [FIX] runbot_merge: missing feedback on fw r+
In some cases, feedback to the PR author that an r+ is redundant went
missing.

This turns out to be due to the convolution of the handling of
approval on forward-port, and the fact that the target PR is treated
exactly like its ancestors: if the PR is already approved the approval
is not even attempted (and so no feedback if it's incorrect).

Straighten up this bit and add a special case for the PR being
commented on, it should have the usual feedback if in error or already
commented on.

Furthermore, update `PullRequests._pr_acl` to kinda work out of the
box for forward-port: if the current PR is a forward port,
`is_reviewer` should check delegation on all ancestors, there doesn't
seem to be any reason to split "source_reviewer", "parent_reviewer",
and "is_reviewer".

Fixes #939
2024-09-05 13:25:19 +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-Do
7411bd5755 [FIX] runbot: allow - in database name
The underscore was previously used in database name, but this is not a
valid character for certificates (still need some investigation about
the limitations)

If it worked to access it it was causing issues for IOT andother.
_ is forbidden on saas actually.

The design_theme database was changed to design-theme, but this is not
accessible through the nginx condig. This should solve the issue.
2024-09-03 10:54:54 +02:00
Xavier-Do
9660cf6517 [IMP] runbot: bundle with all pr closed 2024-08-29 15:03:45 +02:00
Xavier-Do
faef017a25 [FIX] runbot: local cleanup 2024-08-29 08:44:57 +02:00
Christophe Monniez
d722160247 [IMP] drunbot: docker registry
The current runbot infrastructure has 100+ machine that will each build
all docker images.

This is unpractical for multiple reasons:
- impotant load on all machine when the build could be done once
- possible differences for the same image depending on the moment the
base was pulled on the host. An extreme example was the version of
python (3.11 or 3.12) when building the noble docker image before it was
stabilized.
- increase the chance to have a docker build failure in case of network
problem. (random)

A centralized registry will help with that, and allow future devlopment
to generate more docker images. All docker images will be exactly the
same, the pull time is faster than build time, only one build per docker
image, ...
2024-08-29 08:44:57 +02:00
Christophe Monniez
4a2c9d7491 [FIX] runbot: remove useless fields in test 2024-08-28 10:23:29 +02:00
Christophe Monniez
8ac21fb60b [IMP] runbot: allow to archive dockerfile 2024-08-28 10:23:08 +02:00
Christophe Monniez
5fb0e2f5ab [FIX] runbot: avoid builder crash when no docker to build
When there is no Dockerfile marked as `to_build`, the runbot builder
crashes in loop trying to find the max of an empty sequence.

With this commit, the builder does not even try to build Dockerfile in
that case.
2024-08-26 11:04:54 +02:00
Xavier Morel
ccca46c992 [FIX] runbot_merge: layout backend issues galore
- Odoo 17 seems to not be adjusting `nolabel` fields to be `colspan=2`
  by default, so every such occurrence has to be adjusted by hand or
  it gets squeezed in just the labels column.
- Because of the loss of readonly mode, some fields / setups which
  previously looked ugly during the rare edition (e.g. Pr titles) now
  look ugly all the time. Rework layout and force them to always be
  readonly (hopefully we won't need to edit those).
- This is compounded by unfortunate styling I can't find how to
  override e.g. char fields are 100% width even if readonly.
- `<header>` system requires some workarounds to have the right layout
  and spacing (notably `header` has a bunch of awful rules which we
  need to work around via an interstitial div to set up our own
  flexbox).
2024-08-16 15:12:04 +02:00
Xavier Morel
0d653620c2 [FIX] *: styling
- reduce font-size and bold-ness a hair as it causes issues in the
  backend
- remove font adjustment on root object
- add `text-bg-primary` in the oustanding FP view, apparently BS5 does
  not do anything like that by default when setting `bg-primary` for
  some reason so the current team or user appears in black on dark
  blue leading to sub-par readability
2024-08-16 14:20:39 +02:00
Xavier Morel
7eb25234f5 [FIX] runbot_merge: menus
Turns out having the same ids for the feedback template and feedback
object actions was kinda dumb, who'd have thought?

Split them apart so I can get both objects in my menu...
2024-08-16 14:19:13 +02:00
Xavier Morel
8ad186c204 [FIX] runbot_merge: styling on Odoo 17
Fix a few issues with migrated bootstrap classes (left -> start, right
-> end), revert a bunch of shitty colors from standard, fixup
backgrounds.

Tried to remove the background overrides what with having used
variables but it does completely wrong for info, and I can't be
arsed (also force primary alerts for the same reason, I don't
understand what either bootstrap or standard does and how to override
it properly but it's shit). It'll keep.
2024-08-13 14:39:09 +02:00
Xavier Morel
99ae369d69 [FIX] runbot_merge: update freeze wizard widget for 17.0
It was all borken and the web client couldn't even load.

The logic is basically the same, except the new client doesn't support
forcing onchanges maybe, so we need to `read()` the entire thing if
the form is not modified.
2024-08-13 12:50:13 +02:00
Xavier Morel
36786d51c8 [ADD] runbot_merge: 17.0 migration
`message_main_attachment_id` removed from `mail.thread` in
odoo/odoo@f8c7f2e5bb (16.2), however
precise inheritance tracking was only added in
odoo/odoo@b27eb20a41 (17.1), so
stock migration only handles stock models for this migration.
2024-08-13 08:00:21 +02:00
Xavier Morel
aa1df22657 [MERGE] bot from 16.0 to 17.0
Broken (can't run odoo at all):

- In Odoo 17.0, the `pre_init_hook` takes an env, not a cursor, update
  `_check_citext`.
- Odoo 17.0 rejects `@attrs` and doesn't say where they are or how to
  update them, fun, hunt down `attrs={'invisible': ...` and try to fix
  them.
- Odoo 17.0 warns on non-multi creates, update them, most were very
  reasonable, one very wasn't.

Test failures:

- Odoo 17.0 deprecates `name_get` and doesn't use it as a *source*
  anymore, replace overrides by overrides to `_compute_display_name`.
- Multiple tracking changes:
  - `_track_set_author` takes a `Partner` not an id.
  - `_message_compute_author` still requires overriding in order to
    handle record creation, which in standard doesn't support author
    overriding.
  - `mail.tracking.value.field_type` has been removed, the field type
    now needs to be retrieved from the `field_id`.
  - Some tracking ordering have changed and require adjusting a few
    tests.

Also added a few flushes before SQL queries which are not (obviously
at least) at the start of a cron or controller, no test failure
observed but better safe than sorry (probably).
2024-08-12 13:13:03 +02:00
Xavier-Do
2e2eb4e428 [FIX] runbot: move error message to the end
When building a docker image the error is part of the stream, and
at the end.

The current behaviour will append the error message at the begining of
the "result log" breaking the temporality of the output. Adding it at
the end should be more intuitive to read. This will also help to get a
more  usefull error sumary in some cases.
2024-08-12 09:40:48 +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
61b92b2224 [IMP] runbot_merge: use native support for tracking messages
odoo/odoo@1341d52 added native support for tracking / overriding
tracking message so we don't need `change-message `anymore. The calls
had to be modified a bit as `_track_set_log_message` has to be invoked
on the records for which the tracking message is being set / updated.

Sadly the same for authors was only added in 16.3 *and* it's
unsuitable for our needs as we want to set the author without knowing
the scope of affected records (at least in the controller).

That way hopefully in 17.0 we can remove the `_message_compute_author`
override and things should work out. And maybe for 19.0 we can get the
ability to set a per-model (or even global) fallback author into
standard.
2024-08-09 10:10:01 +02:00
Xavier Morel
f378689a0d [FIX] runbot_merge: dumbshit 16.0 view validation complaints 2024-08-09 10:09:50 +02:00
Xavier Morel
a567960d52 [FIX] runbot_merge: 16.0 orm warning
Regular indexing doesn't work if unaccent is enabled, and the ORM
warns about it.
2024-08-09 10:08:36 +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