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.

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

CI: Migrate test-new to meson #39641

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
183 changes: 2 additions & 181 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,187 +76,8 @@ env:
EXTRA_CONFIGURE_ARGS: --enable-fat-binary

jobs:
test-new:
runs-on: ubuntu-latest
outputs:
build_targets: ${{ steps.build-targets.outputs.build_targets }}
services:
# https://docs.docker.com/build/ci/github-actions/local-registry/
registry:
image: registry:2
ports:
- 5000:5000
steps:
- name: Maximize build disk space
uses: easimon/maximize-build-space@v10
with:
# need space in /var for Docker images
root-reserve-mb: 30000
remove-dotnet: true
remove-android: true
remove-haskell: true
remove-codeql: true
remove-docker-images: true

- name: Checkout
id: checkout
uses: actions/checkout@v4

- name: Get changed files and packages
id: changed-files
uses: tj-actions/changed-files@v45
with:
# File extensions for doctests per sage.doctest.control.skipfile
files_yaml: |
configures:
- 'build/pkgs/*/spkg-configure.m4'
pkgs:
- 'build/pkgs/**'
- '!build/pkgs/_**'
- '!build/pkgs/configure/**'
- 'pkgs/**'
doctests:
- 'src/**/*.{py,pyx,pxd,pxi,sage,spyx,rst,tex}'
- '!src/{setup,conftest*}.py'

- name: Determine targets to build
id: build-targets
run: |
uninstall_targets=$(echo $(
for a in '' ${{ steps.changed-files.outputs.configures_all_changed_files }}; do
# Extract package name from the file path and append '-uninstall'
echo $a | sed -E 's,build/pkgs/([a-z0-9][_.a-z0-9]*)/spkg-configure[.]m4 *,\1-uninstall,'
done | sort -u # Sort and ensure uniqueness
))
build_targets=$(echo $(
for a in '' ${{ steps.changed-files.outputs.pkgs_all_changed_files }}; do
# Extract package name, replace '-' with '_', and strip extra parts from the path
SPKG=$(echo $a | sed -E 's,-,_,g;s,(build/)?pkgs/([a-z0-9][-_.a-z0-9]*)/[^ ]* *,\2,;')
# Check if key files exist in the package directory
if [ -f "build/pkgs/$SPKG/checksums.ini" ] || \
[ -f "build/pkgs/$SPKG/requirements.txt" ] || \
[ -f "build/pkgs/$SPKG/spkg-install" ]; then
echo "$SPKG-ensure" # add the "$SPKG-ensure" target
fi
done | sort -u # Sort and ensure uniqueness
))
if [ -n "$uninstall_targets" ]; then
echo "build_targets=$uninstall_targets reconfigure $build_targets ci-build-with-fallback" >> $GITHUB_OUTPUT
else
echo "build_targets=$build_targets ci-build-with-fallback" >> $GITHUB_OUTPUT
fi
cat $GITHUB_OUTPUT

- uses: actions/checkout@v4
with:
ref: ${{ github.base_ref }}
path: worktree-base
if: github.base_ref && steps.changed-files.outputs.pkgs_all_changed_files

- name: Compute metrics
run: |
export PATH=build/bin:$PATH
if [ -d worktree-base ]; then
(echo "# $GITHUB_BASE_REF"; SAGE_ROOT=worktree-base sage-package metrics :all:) > base-metrics.txt
(echo "# $GITHUB_REF"; sage-package metrics :all:) > metrics.txt
diff --color=always --width=100 --side-by-side --left-column base-metrics.txt metrics.txt || true
else
sage-package metrics :all:
fi

- name: Install test prerequisites
# From docker.yml
run: |
sudo DEBIAN_FRONTEND=noninteractive apt-get update
sudo DEBIAN_FRONTEND=noninteractive apt-get install tox
sudo apt-get clean
df -h

- name: Merge CI fixes from sagemath/sage
# From docker.yml
# This step needs to happen after the commit sha is put in DOCKER_TAG
# so that multi-stage builds can work correctly.
run: |
mkdir -p upstream
.ci/merge-fixes.sh 2>&1 | tee upstream/ci_fixes.log
env:
GH_TOKEN: ${{ github.token }}
SAGE_CI_FIXES_FROM_REPOSITORIES: ${{ vars.SAGE_CI_FIXES_FROM_REPOSITORIES }}

# Building

- name: Generate Dockerfile
# From docker.yml
run: |
tox -e ${{ env.TOX_ENV }}
cp .tox/${{ env.TOX_ENV }}/Dockerfile .
env:
# Only generate the Dockerfile, do not run 'docker build' here
DOCKER_TARGETS: ""

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
with:
driver-opts: network=host

- name: Build Docker image
id: image
uses: docker/build-push-action@v6
with:
# push and load may not be set together at the moment
#
# We are using "push" (to the local registry) because it was
# more reliable than "load", for which we observed random failure
# conditions in which the built image could not be found.
#
push: ${{ steps.changed-files.outputs.doctests_all_changed_files && true || false }}
load: false
context: .
tags: ${{ env.BUILD_IMAGE }}
target: with-targets
cache-from: type=gha
cache-to: type=gha,mode=max
build-args: |
NUMPROC=6
USE_MAKEFLAGS=-k V=0 SAGE_NUM_THREADS=4 --output-sync=recurse
TARGETS_PRE=build/make/Makefile
TARGETS=${{ steps.build-targets.outputs.build_targets }}

- name: Start container
id: container
# Try to continue when "exporting to GitHub Actions Cache" failed with timeout
if: (success() || failure()) && steps.changed-files.outputs.doctests_all_changed_files
run: |
docker run --name BUILD -dit \
--mount type=bind,src=$(pwd),dst=$(pwd) \
--workdir $(pwd) \
${{ env.BUILD_IMAGE }} /bin/sh

# Testing

- name: Check that all modules can be imported
if: (success() || failure()) && steps.container.outcome == 'success' && steps.changed-files.outputs.doctests_all_changed_files
run: |
# Increase the length of the lines in the "short summary"
export COLUMNS=120
# The following command checks that all modules can be imported.
# The output also includes a long list of modules together with the number of tests in each module.
# This can be ignored.
./sage -python -m pip install pytest-xdist
./sage -python -m pytest -c tox.ini -qq --doctest --collect-only || true
shell: sh .ci/docker-exec-script.sh BUILD /sage {0}

- name: Test changed files (sage -t --new)
if: (success() || failure()) && steps.container.outcome == 'success' && steps.changed-files.outputs.doctests_all_changed_files
run: |
export MAKE="make -j2 --output-sync=recurse" SAGE_NUM_THREADS=4
# https://github.com/tj-actions/changed-files?tab=readme-ov-file#outputs-
./sage -t --long --format github -p4 ${{ steps.changed-files.outputs.doctests_all_changed_files }}
shell: sh .ci/docker-exec-script.sh BUILD /sage {0}

test-long:
runs-on: ubuntu-latest
needs: [test-new]
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't remove this defeat the whole point of test-new? (that you do a fast-fail when there's a trivial error to notify the pull request author to fix it, so you don't burn CPU on testing the rest of the files)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main point of test-new is that it provides quick feedback for devs. But I'm planning to also move these "long" tests to meson in a follow-up and then it's indeed a good idea to make run only after all other normal tests were passing.

services:
# https://docs.docker.com/build/ci/github-actions/local-registry/
registry:
Expand Down Expand Up @@ -324,7 +145,7 @@ jobs:
id: image
uses: docker/build-push-action@v6
with:
push: true
push: false
load: false
context: .
tags: ${{ env.BUILD_IMAGE }}
Expand Down Expand Up @@ -444,7 +265,7 @@ jobs:
id: image
uses: docker/build-push-action@v6
with:
push: true
push: false
load: false
context: .
tags: ${{ env.BUILD_IMAGE }}
Expand Down
36 changes: 33 additions & 3 deletions .github/workflows/ci-meson.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,24 @@ concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

permissions:
contents: read

jobs:
test:
name: Conda (${{ matrix.os }}, Python ${{ matrix.python }})
name: Conda (${{ matrix.os }}, Python ${{ matrix.python }}, ${{ matrix.tests }})
runs-on: ${{ matrix.os }}-latest

strategy:
fail-fast: false
matrix:
os: [ubuntu]
os: ['ubuntu']
python: ['3.11', '3.12']
tests: ['all']
include:
- os: 'ubuntu'
python: '3.12'
tests: 'new'

steps:
- uses: actions/checkout@v4
Expand All @@ -34,6 +42,17 @@ jobs:
env:
GH_TOKEN: ${{ github.token }}

- name: Mark new files as uncommited
if: matrix.tests == 'new'
run: |
# List remotes (for debugging)
git remote -v
# Reset the branch to develop
git fetch origin develop
git reset --soft origin/develop
# Show uncommitted changes
git status

- name: Cache conda packages
uses: actions/cache@v4
with:
Expand Down Expand Up @@ -76,6 +95,7 @@ jobs:
# this step must be after build, because meson.build creates a number of __init__.py files
# that is needed to make tools/update-meson.py run correctly
shell: bash -l {0}
if: matrix.tests == 'all'
run: |
python3 tools/update-meson.py
if ! ./tools/test-git-no-uncommitted-changes; then
Expand All @@ -94,7 +114,17 @@ jobs:
run: |
# We don't install sage_setup, so don't try to test it
rm -R ./src/sage_setup/
./sage -t --all -p4 --format github
./sage -t --${{ matrix.tests }} -p4 --format github

- name: Check that all modules can be imported
shell: bash -l {0}
run: |
# Increase the length of the lines in the "short summary"
export COLUMNS=120
# The following command checks that all modules can be imported.
# The output also includes a long list of modules together with the number of tests in each module.
# This can be ignored.
pytest -qq --doctest --collect-only

- name: Upload log
uses: actions/[email protected]
Expand Down
Loading