Skip to content

Add a test using its own executor (until the Kani library includes one)#1658

Merged
danielsn merged 5 commits intomodel-checking:mainfrom
fzaiser:spawn
Sep 17, 2022
Merged

Add a test using its own executor (until the Kani library includes one)#1658
danielsn merged 5 commits intomodel-checking:mainfrom
fzaiser:spawn

Conversation

@fzaiser
Copy link
Contributor

@fzaiser fzaiser commented Sep 8, 2022

This adds a test involving an executor written by the user. Long term, we want such an executor to be in the standard library but this is not currently possible because of linking issues.

Related issues:

Related: #1504

Testing:

  • How is this change tested? It's an added test

  • Is this a refactor change? No

Checklist

  • Each commit message has a non-empty body, explaining why the change was made
  • Methods or procedures are documented
  • Regression or unit tests are included, or existing tests cover the modified code
  • My PR is restricted to a single feature or bugfix

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT licenses.

@fzaiser fzaiser requested a review from a team as a code owner September 8, 2022 20:38
}

/// Adds a future to the scheduler's task list, returning a JoinHandle
pub fn spawn<F: Future<Output = ()> + Sync + 'static>(&mut self, fut: F) -> JoinHandle {
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if you spawn more than MAX_TASKS

Copy link
Contributor

Choose a reason for hiding this comment

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

Make an explicit test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add such a test to the libary PR, but since the number of tasks is always two in the test case in this file, I don't think this warrants the additional complexity.


#[kani::proof]
#[kani::unwind(4)]
fn deterministic_schedule() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This name is surprising

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

}

#[kani::proof]
#[kani::unwind(4)]
Copy link
Contributor

Choose a reason for hiding this comment

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

How long does this take?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

About 15 seconds on my machine.

@danielsn
Copy link
Contributor

danielsn commented Sep 9, 2022

Approved modulo comments

@fzaiser
Copy link
Contributor Author

fzaiser commented Sep 17, 2022

@danielsn Can you merge this please?

@danielsn danielsn merged commit 61c8d11 into model-checking:main Sep 17, 2022
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.

2 participants