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

Revert test matrix back to manual #111

Merged
merged 37 commits into from
May 27, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
1e50018
check tests fail
lilyminium Apr 7, 2024
51d4010
update job name
lilyminium Apr 18, 2024
0d44293
reset name back to main-tests
lilyminium Apr 21, 2024
8fb7da0
rm dev
lilyminium Apr 21, 2024
d744ba6
change python matrix to manual
lilyminium Apr 21, 2024
102549c
is codecov the problem here?
lilyminium Apr 22, 2024
a62d745
dont fail ci for codecov
lilyminium Apr 22, 2024
c9de494
remove all dynamic python versions
lilyminium Apr 27, 2024
16324a5
add codecov token?
lilyminium Apr 27, 2024
d449d2d
escape braces
lilyminium Apr 27, 2024
bfa4b66
ignore codecov failure?
lilyminium May 26, 2024
5186c18
upgrade act and allow codecov failure again
lilyminium May 26, 2024
007e0a1
add more comment to CI
lilyminium May 26, 2024
57050d2
try changing the name of the package
lilyminium May 26, 2024
956b25d
Revert "try changing the name of the package"
lilyminium May 26, 2024
66a4245
assert False
lilyminium May 26, 2024
6b36c22
tmp list directory
lilyminium May 26, 2024
b860ccf
downgrade act?
lilyminium May 26, 2024
2601768
rm assert False
lilyminium May 26, 2024
6ffb6c5
pass secret to act
lilyminium May 26, 2024
efea53e
add actions workaround for secret
lilyminium May 26, 2024
9c7a1dd
try oidc?
lilyminium May 27, 2024
ceec4a3
give up and try editing file to ignore upload failure
lilyminium May 27, 2024
51bdcff
Revert "rm assert False"
lilyminium May 27, 2024
537f817
Revert "Revert "rm assert False""
lilyminium May 27, 2024
0abac8a
Fix CI to fail and run appropriately (#116)
lilyminium May 27, 2024
fcb4c12
try v4
lilyminium May 27, 2024
3dd02ed
bump act action?
lilyminium May 27, 2024
2ecd7b0
upgrade versions
lilyminium May 27, 2024
c2109f4
why isnt this running?
lilyminium May 27, 2024
38b8320
reorder os?
lilyminium May 27, 2024
ca96eaf
upgrade act all the way
lilyminium May 27, 2024
ba6795f
its called act-latest now?
lilyminium May 27, 2024
e879faf
oops wrong place
lilyminium May 27, 2024
c871ad8
add p=false
lilyminium May 27, 2024
e1d47d5
downgrade codecov back to v4, act back to 0.2.40, see how that goes
lilyminium May 27, 2024
a685a34
Revert "upgrade versions"
lilyminium May 27, 2024
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
7 changes: 6 additions & 1 deletion .github/actions/run-cookie-ci/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ inputs:
description: email associated with GitHub user
required: true
default: github-action@users.noreply.github.com
CODECOV_TOKEN:
description: Codecov token
required: true

runs:
using: "composite"
Expand Down Expand Up @@ -54,4 +57,6 @@ runs:
shell: bash
run: |
docker images
act --job main-tests --platform ubuntu-latest=act-conda
sed -i -e 's/fail_ci_if_error: true/fail_ci_if_error: false/g' .github/workflows/gh-ci.yaml
cat .github/workflows/gh-ci.yaml
act --job main-tests --platform ubuntu-latest=act-conda -s CODECOV_TOKEN=${{ inputs.CODECOV_TOKEN }}
12 changes: 10 additions & 2 deletions .github/workflows/gh-ci.yaml
Copy link
Member

Choose a reason for hiding this comment

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

If you can be bothered, there's a few action versions that could be bumped up here (upload, download, and checkout can all go to v4, setup-miniconda I think would be good to bump to v3 [I know it doesn't play nice with macos-latest osx-arm64 runners])

Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ defaults:
env:
ARTIFACT_NAME: demo-mdakit-repos
OUTPUT_DIRECTORY: /home/runner/example_outputs
OUTPUT_COOKIE_SUBDIRECTORY: TestMDAKit_with_host_MDAnalysis_condaforge-deps_and_ReadTheDocs/mdakit-cookie
GH_USER: github-actions
GH_EMAIL: "github-action@users.noreply.github.com"
GH_REPOSITORY: "github.com/${{ github.repository }}.git"
Expand Down Expand Up @@ -60,19 +61,26 @@ jobs:
run: |
# --keep-test-outputs also saves the generated repositories
if [[ ${{ matrix.os }} == "ubuntu-latest" ]] ; then
PYTEST_FLAGS="--keep-test-outputs $OUTPUT_DIRECTORY"
PYTEST_FLAGS="--keep-test-outputs ${{ env.OUTPUT_DIRECTORY }}"
else
PYTEST_FLAGS=""
fi
echo "PYTEST_FLAGS=${PYTEST_FLAGS}"

pytest tests/ $PYTEST_FLAGS

- name: check cookie CI presence
if: ${{ matrix.os == 'ubuntu-latest' }}
run: |
ls -la ${{ env.OUTPUT_DIRECTORY }}/
IAlibay marked this conversation as resolved.
Show resolved Hide resolved
ls -la ${{ env.OUTPUT_DIRECTORY }}/${{ env.OUTPUT_COOKIE_SUBDIRECTORY }}/

- name: Run cookie CI
if: ${{ matrix.os == 'ubuntu-latest' }}
uses: ./.github/actions/run-cookie-ci
with:
source-directory: ${{ env.OUTPUT_DIRECTORY }}/TestMDAKit_with_host_MDAnalysis_condaforge-deps_and_ReadTheDocs/mdakit-cookie
source-directory: ${{ env.OUTPUT_DIRECTORY }}/${{ env.OUTPUT_COOKIE_SUBDIRECTORY }}
CODECOV_TOKEN: ${{ secrets.COOKIE_CODECOV_TOKEN }}

- name: Upload artifact
if: ${{ matrix.os == 'ubuntu-latest' && matrix.last-n-minor-python-release == 0 }}
Expand Down
2 changes: 1 addition & 1 deletion tests/test_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def test_write_outputs(
kitter = CookiecutterMDAKit(
project_name=project_name,
repo_name="mdakit-cookie",
package_name="mdakit_cookie",
package_name="cookiekit",
github_username="test-user-account",
github_host_account=github_host_account,
description=description,
Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

Something I had to do with the broken kits was bump up the conda-incubator/setup-miniconda version.

I found a lot of times that v2 would fail with osx-arm64 for some reason or another.

Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,21 @@ defaults:
shell: bash {% if cookiecutter.__dependency_source != 'pip' %} -l {0} {% endif %}

jobs:
environment-config:
runs-on: ubuntu-latest
outputs:
stable-python-version: {{ "${{ steps.get-compatible-python.outputs.stable-python }}" }}
python-matrix: {{ "${{ steps.get-compatible-python.outputs.python-versions }}" }}
lilyminium marked this conversation as resolved.
Show resolved Hide resolved
lilyminium marked this conversation as resolved.
Show resolved Hide resolved
steps:
- uses: actions/setup-python@v4
with:
python-version: "3.11"

- id: get-compatible-python
uses: MDAnalysis/mdanalysis-compatible-python@main
with:
release: "latest"

main-tests:
if: "github.repository == '{{ cookiecutter.github_host_account }}/{{ cookiecutter.repo_name }}'"
needs: environment-config
runs-on: {{ '${{ matrix.os }}' }}
strategy:
fail-fast: false
matrix:
os: [macOS-latest, ubuntu-latest, windows-latest]
python-version: {% raw %}${{ fromJSON(needs.environment-config.outputs.python-matrix) }}{% endraw %}
mdanalysis-version: ["latest", "develop"]
python-version: ["3.10", "3.11", "3.12"]
exclude:
# Entries here exclude particular combinations of the matrix
# Edit or remove as particular combinations come into or out of date
# Below we exclude runs with the latest release and Python 3.12
- mdanalysis-version: "latest"
python-version: "3.12"

steps:
- uses: actions/checkout@v4
Expand Down Expand Up @@ -123,11 +113,22 @@ jobs:
file: coverage.xml
name: codecov-{{ '${{ matrix.os }}' }}-py{{ '${{ matrix.python-version }}' }}
verbose: True
# to upload coverage reports, set a secret called CODECOV_TOKEN
Copy link
Member

Choose a reason for hiding this comment

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

Do we have a set of setup instruction docs somewhere where we can put this? (not in this PR but let's raise an issue to get that done at some point :) )

# in the repository settings
# (Obtain this from the Codecov website after setting up the repository there)
token: {{ '${{ secrets.CODECOV_TOKEN }}' }}
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a thing with codecov v4 about this - not sure if we should/want to bump up the version (it might make things break even more!)

# To fail the CI if there's an error, keep this set to true
# If repository forks need to run CI, you may need to set this to false
# Forks can't access the CODECOV_TOKEN secret,
# and a failed upload registers as an error
fail_ci_if_error: true
# use OpenID Connect (OIDC) if you have this enabled -- this will ignore
# the `token` argument above
use_oidc: true
Copy link
Member

Choose a reason for hiding this comment

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

Are the cookiecutter tests currently failing because this is set to true?

Copy link
Member

Choose a reason for hiding this comment

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

Follow up: should this be true by default?

Copy link
Member

Choose a reason for hiding this comment

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

Probably related; TIL use_oidc is v4 only.

Copy link
Member Author

@lilyminium lilyminium May 27, 2024

Choose a reason for hiding this comment

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

They're currently failing because of the issues solved in #115! I did test that the fix in #115 resolves this. The codecov action currently gets edited on the fly so failure is ignored.

Probably related; TIL use_oidc is v4 only.

Interesting, the logs seemed to indicate that codecov was picking up the OIDC token and using it to connect.

Follow up: should this be true by default?

I would assume that OIDC is a better means of validation than the plain CODECOV token, if users can do so -- that being said I'm not quite sure how it well it works for forks. I gave it a try just in case it magically helped the upload. Happy to remove and look at it more in detail later for a simpler PR.



pylint_check:
if: "github.repository == '{{ cookiecutter.github_host_account }}/{{ cookiecutter.repo_name }}'"
needs: environment-config
runs-on: ubuntu-latest

steps:
Expand All @@ -136,7 +137,7 @@ jobs:
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: {{ '${{ needs.environment-config.outputs.stable-python-version }}' }}
python-version: "3.11"

- name: Install Pylint
run: |
Expand All @@ -153,16 +154,15 @@ jobs:

pypi_check:
if: "github.repository == '{{ cookiecutter.github_host_account }}/{{ cookiecutter.repo_name }}'"
needs: environment-config
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4

- name: Set up Python {{ '${{ needs.environment-config.outputs.stable-python-version }}' }}
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: {{ '${{ needs.environment-config.outputs.stable-python-version }}' }}
python-version: "3.11"

- name: Install dependencies
run: |
Expand Down
Loading