Skip to content
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

Behavior regression CI #1642

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Behavior regression CI #1642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d220103
error-regression
3Rafal Jun 28, 2023
bad37c8
Fix branch name
3Rafal Jun 29, 2023
210ce0c
Update results to reflect new version of merl-an
3Rafal Jun 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions .github/workflows/regression.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
name: Regression

on:
pull_request:
push:
branches:
- master
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should restrict the ci's run to changes made to .ml / .mli files


jobs:
error-regression:
name: Error regression

strategy:
fail-fast: false
matrix:
os:
- ubuntu-latest
ocaml-compiler:
- 4.14.x
3Rafal marked this conversation as resolved.
Show resolved Hide resolved

runs-on: ${{ matrix.os }}

steps:
- name: Checkout code
uses: actions/checkout@v3
with:
submodules: true

- name: Use OCaml ${{ matrix.ocaml-compiler }}
uses: ocaml/setup-ocaml@v2
with:
ocaml-compiler: ${{ matrix.ocaml-compiler }}
dune-cache: ${{ matrix.os == 'ubuntu-latest' }}

- name: Install opam packages
run: opam install --deps-only .

- name: Run build
run: opam exec -- dune build

- name: Install merlin
run: opam install .

- name: Install merl-an
run: opam pin -y merl-an https://github.com/pitag-ha/merl-an.git

- name: Run tests
run: REGRESSION=true opam exec -- dune test regression
7 changes: 7 additions & 0 deletions regression/build-irmin
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash
git clone https://github.com/mirage/irmin.git
cd irmin
git checkout 8da4d16e7cc8beddfc8a824feca325426bae08a9
sudo apt install -y gnuplot-x11 libgmp-dev pkg-config libffi-dev
opam install . --deps-only --with-test --no-checksums -y
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
opam install . --deps-only --with-test --no-checksums -y
opam install . --deps-only --with-test -y

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

one of the irmin deps (duff) fail on checksum, so I had to make it this way.

opam exec -- dune build
9 changes: 9 additions & 0 deletions regression/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
(env (_
(binaries build-irmin)))
Comment on lines +1 to +2
Copy link
Member

Choose a reason for hiding this comment

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

Do you happen to know if the build-irmin result gets cached? It takes about 5 min, so it's not a huge deal if it doesn't. If it's easy to do (or already the case), it would still be nice though, among others to reduce the cost of adding more source code than Irmin to test on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, it doesn't. big parts of our pipelines aren't cached. I don't know about easy way to do this, but I agree it would be nice.

Copy link
Member

Choose a reason for hiding this comment

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

I don't know about easy way to do this, but I agree it would be nice.

If we had the script be called by the GH action rather than by Dune, could we make use of GH action caching? (It's a genuine question. I don't know myself either.)

Copy link
Collaborator Author

@3Rafal 3Rafal Jun 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Running this script via GH actions / dune seems orthogonal to caching. I run it via dune, because checking out Irmin code in test directory makes dune test run Irmin tests as well. This could probably be fixed, but I didn't want to spend too much time on this.

Caching build dependency can be done via GH action, but I'm not sure about an actual implementation. It would be a nice optimisation, but realistically we would shave couple of minutes from the build job that currently takes ~40 minutes. I don't think it 's the most valuable thing to spend time on at this moment.


(cram
(enabled_if
(= true %{env:REGRESSION=false}))
3Rafal marked this conversation as resolved.
Show resolved Hide resolved
(applies_to :whole_subtree)
(deps
%{bin:build-irmin}))
Loading