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

added forceDepthFirst flag to closure and closureStar for testing purposes #237

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

Conversation

jurgenvinju
Copy link
Member

This does not terminate while running the tests yet. Probably an issue with the IBool value generator?

* @throws UnsupportedOperationException when the receiver is not a binary relation
*/
public default C closure() {
public default C closure(boolean forceDepthFirst) {
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 need to break our public contract for this? How about still having the regular function in here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, yes we should have a default version.

Copy link
Member

@DavyLandman DavyLandman Jan 13, 2024

Choose a reason for hiding this comment

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

how about we give it an enum? or a way to say: auto vs DFS/BFS? so a user can say: hey, this is BFS, so let's go for that. Since even for big collections, there are graphs where BFS is better.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is only for testing purposes. We can never give this to the rascal user anyway. And to be honest, there are dozens of algorithms we still have to try. It's quite possible that the distinction between DFS and bfs becomes a lot more vague.

Copy link

Test Results

     55 files   -      41       55 suites   - 41   3m 3s ⏱️ - 4m 12s
177 875 tests  -  64 414  177 874 ✅  -  64 414  1 💤 ±0  0 ❌ ±0 
368 540 runs   - 358 414  368 538 ✅  - 358 413  2 💤  - 1  0 ❌ ±0 

Results for commit 849c655. ± Comparison against base commit 073dbdd.

This pull request removes 64614 and adds 200 tests. Note that renamed tests count towards both.
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory)[1]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory)[2]
io.usethesource.vallang.specification.IRelationTests ‑ broadClosure(IValueFactory)[1]
io.usethesource.vallang.specification.IRelationTests ‑ broadClosure(IValueFactory)[2]
io.usethesource.vallang.specification.IRelationTests ‑ carrierForTriples(ISet)[100]
io.usethesource.vallang.specification.IRelationTests ‑ carrierForTriples(ISet)[101]
io.usethesource.vallang.specification.IRelationTests ‑ carrierForTriples(ISet)[102]
io.usethesource.vallang.specification.IRelationTests ‑ carrierForTriples(ISet)[103]
io.usethesource.vallang.specification.IRelationTests ‑ carrierForTriples(ISet)[104]
io.usethesource.vallang.specification.IRelationTests ‑ carrierForTriples(ISet)[105]
…
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[100]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[101]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[102]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[103]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[104]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[105]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[106]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[107]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[108]
io.usethesource.vallang.basic.RelationSmokeTest ‑ testClosure(IValueFactory, IBool)[109]
…

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.

None yet

2 participants