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

Test lower compat bounds on LTS #380

Closed
wants to merge 15 commits into from
Closed

Test lower compat bounds on LTS #380

wants to merge 15 commits into from

Conversation

adrhill
Copy link
Collaborator

@adrhill adrhill commented Jul 25, 2024

Closes #378.

Uses the julia-downgrade-compat on the LTS tests to test on the lower bounds of compat-entries.

Note that this PR puts some constraints on future compat entries:

Supported compat entries

Compats like 1, 1.2, 1.2.3, ^1.2.3, ~1.2.3, =1.2.3, 1.2.3, 2.3.4 are all supported.

Compats like 1.2.3 - 1.2.5 are not supported.

For list compats like 1.2.3, 2.3.4, all but the first entry is ignored. Therefore you should put the lowest entry first.

@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.58%. Comparing base (0f7957d) to head (25b7c1e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #380      +/-   ##
==========================================
- Coverage   96.62%   96.58%   -0.05%     
==========================================
  Files         102      102              
  Lines        4974     4974              
==========================================
- Hits         4806     4804       -2     
- Misses        168      170       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@adrhill
Copy link
Collaborator Author

adrhill commented Jul 25, 2024

I'm not sure whether this action should be called before or after the cache action.
I opened an issue in julia-downgrade-compat:
julia-actions/julia-downgrade-compat#9

@gdalle
Copy link
Member

gdalle commented Jul 25, 2024

Thanks for this attempt, but see the comment #378 (comment) for why this is bound to fail: the AD backends are not part of the DI test environment, they are added afterwards (and so is DIT).

@adrhill
Copy link
Collaborator Author

adrhill commented Jul 25, 2024

I don't think the problem is unfixable, but let's not allow perfection to get into the way of progress:

How about reverting the general LTS tests and adding tests on lower compat bounds to Internals tests and Zero backend tests only?

@gdalle
Copy link
Member

gdalle commented Jul 25, 2024

I think downgrade compat should work once #381 is merged.

However I don't see why you would need to remove the precise compat entries that I designed with great care ^^ There is a reason behind most of those, and things will fail if you remove them.

@adrhill
Copy link
Collaborator Author

adrhill commented Jul 25, 2024

However I don't see why you would need to remove the precise compat entries that I designed with great care ^^

I guessed this would be the case, but I wanted to see these lower bound tests fail to make sure they are working. ;)

@gdalle
Copy link
Member

gdalle commented Jul 25, 2024

I guessed this would be the case, but I wanted to see these lower bound tests fail to make sure they are working. ;)

If you want a surefire check, anything strictly less than SparseMatrixColorings v0.3.4 will error right away in DI because one of the names I use was defined in v0.3.4

@gdalle
Copy link
Member

gdalle commented Jul 25, 2024

Another issue is that only a handful of backends get tested on LTS. Ideally we'd wanna run these tests on the stable release of Julia (too).
But I don't wanna run into https://docs.github.com/en/actions/administering-github-actions/usage-limits-billing-and-administration

@adrhill
Copy link
Collaborator Author

adrhill commented Jul 25, 2024

Maybe the solution that's least expensive in GitHub-Actions-compute is to test all backends on LTS?

@adrhill
Copy link
Collaborator Author

adrhill commented Jul 25, 2024

Looks like this caught an actual bug in SparseMatrixColorings.jl:

ERROR: LoadError: UndefVarError: stack not defined
Stacktrace:
 ...
in expression starting at /home/runner/.julia/packages/SparseMatrixColorings/AwBlN/src/SparseMatrixColorings.jl:1
ERROR: LoadError: Failed to precompile SparseMatrixColorings [0a514795-09f3-496d-8182-132a7b665d35] to /home/runner/.julia/compiled/v1.6/SparseMatrixColorings/jl_uGzo2C.

https://github.com/gdalle/SparseMatrixColorings.jl/blob/83a1330a025ba2c45b95e144c388380ffec1ace1/src/SparseMatrixColorings.jl#L10

EDIT: Opened gdalle/SparseMatrixColorings.jl#32

@gdalle gdalle closed this Jul 30, 2024
@gdalle gdalle reopened this Jul 30, 2024
@gdalle gdalle marked this pull request as draft July 30, 2024 14:05
@adrhill adrhill closed this Aug 1, 2024
@gdalle gdalle deleted the ah/downgrade-tests branch September 24, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test lower compat bounds?
3 participants