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

Tweak exterior_algebra tests #3415

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

fingolfin
Copy link
Member

Do not just verify that certain exterior_algebra invocations don't error out; also check that the returned values satisfy certain properties.

Also disable two @test_broken tests for now, because (a) the underlying issue is fixed in a Singular PR, but this causes OscarCI failures there.. (b) even if the underlying issue is fixed, those test were wrong, because exterior_algebra doesn't return a boolean, but the return value was used in a context where it must be boolean (namely as "argument" for @test_broken).

I hope we can quickly merge this, then we can re-run the oscar-system/Singular.jl#773 CI tests. If those pass, we'd merge that into Singular.jl, release Singular.jl -- then finally we could enable the two currently commented out (former @test_broken) tests.

CC @hannes14

Do not just verify that certain `exterior_algebra` invocations don't error
out; also check that the returned values satisfy certain properties.

Also disable two `@test_broken` tests for now, because (a) the underlying
issue is fixed in a Singular PR, but this causes OscarCI failures there.. (b)
even if the underlying issue is fixed, those test were wrong, because
`exterior_algebra` doesn't return a boolean, but the return value was used in
a context where it must be boolean (namely as "argument" for `@test_broken`).
@fingolfin fingolfin added the backport 1.0.x Should be backported to the release 1.0 branch label Feb 22, 2024
Copy link
Member

@hannes14 hannes14 left a comment

Choose a reason for hiding this comment

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

g is
n-element Vector{PBWAlgQuoElem{...}}:
e1
e2...
which gives:
gens(g)
ERROR: MethodError: no method matching gens
same for ngens

@benlorenz benlorenz mentioned this pull request Feb 23, 2024
33 tasks
@fingolfin fingolfin merged commit f7e6c79 into oscar-system:master Feb 23, 2024
20 checks passed
@fingolfin fingolfin deleted the mh/exterior_algebra branch February 23, 2024 08:45
benlorenz pushed a commit that referenced this pull request Feb 23, 2024
Do not just verify that certain `exterior_algebra` invocations don't error
out; also check that the returned values satisfy certain properties.

Also disable two `@test_broken` tests for now, because (a) the underlying
issue is fixed in a Singular PR, but this causes OscarCI failures there.. (b)
even if the underlying issue is fixed, those test were wrong, because
`exterior_algebra` doesn't return a boolean, but the return value was used in
a context where it must be boolean (namely as "argument" for `@test_broken`).
@benlorenz benlorenz removed the backport 1.0.x Should be backported to the release 1.0 branch label Feb 23, 2024
benlorenz added a commit that referenced this pull request Feb 23, 2024
### Backported PRs

- [x] #3367
- [x] #3379 
- [x] #3369
- [x] #3291
- [x] #3325
- [x] #3350 
- [x] #3351
- [x] #3365 
- [x] #3366
- [x] #3382
- [x] #3373
- [x] #3341
- [x] #3346
- [x] #3381
- [x] #3385
- [x] #3387 
- [x] #3398 
- [x] #3399 
- [x] #3374 
- [x] #3406 
- [x] #2823
- [x] #3298
- [x] #3386 
- [x] #3412 
- [x] #3392 
- [x] #3415 
- [x] #3394
- [x] #3391
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.

4 participants