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

Emit an error if Kani executable was not found #79

Merged
merged 3 commits into from
May 17, 2023

Conversation

zhassan-aws
Copy link
Contributor

@zhassan-aws zhassan-aws commented May 16, 2023

Description of changes:

Check if which cargo-kani returned an error, abort execution, and set the test result as "errored". This is what gets shown in VSCode:

image

Resolved issues:

Resolves #72

Related RFC:

Optional #ISSUE-NUMBER.

Call-outs:

A cleaner approach would be to check on load if Kani is installed, and if not put the extension in an error state and prevent running any harnesses.

Testing:

  • How is this change tested? Removed kani from path, launched extension, and ran one of the harnesses.

  • Is this a refactor change?

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.

Copy link
Contributor

@jaisnan jaisnan left a comment

Choose a reason for hiding this comment

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

The changes look great! I do think the cleaner solution can be added easily as well on top of these changes. It should be simple to implement as we already do exactly the same, with cargo here - https://github.com/model-checking/kani-vscode-extension/blob/15c27c756b029833d126d12b4032b9c5aa1e6175/src/extension.ts#LL34C3-L34C3.

@jaisnan
Copy link
Contributor

jaisnan commented May 17, 2023

Could I ask if you could add a screenshot of what the end result looks like visually? And also what underlying environment conditions allowed you to trigger the issue (i.e what was the cargo-kani path in your case). Any information on how it was tested would be helpful to me.

Thanks!

@zhassan-aws
Copy link
Contributor Author

Could I ask if you could add a screenshot of what the end result looks like visually? And also what underlying environment conditions allowed you to trigger the issue (i.e what was the cargo-kani path in your case). Any information on how it was tested would be helpful to me.

I added a script to the PR description as well as how I tested it. In my case, I removed kani from the PATH environment variable, which causes which cargo-kani to exit with an error status.

@jaisnan
Copy link
Contributor

jaisnan commented May 17, 2023

Could I ask if you could add a screenshot of what the end result looks like visually? And also what underlying environment conditions allowed you to trigger the issue (i.e what was the cargo-kani path in your case). Any information on how it was tested would be helpful to me.

I added a script to the PR description as well as how I tested it. In my case, I removed kani from the PATH environment variable, which causes which cargo-kani to exit with an error status.

Thanks for the description. The final change looks great too.

@zhassan-aws
Copy link
Contributor Author

I do think the cleaner solution can be added easily as well on top of these changes. It should be simple to implement as we already do exactly the same, with cargo here

I couldn't get this approach to work. My approach is in this branch. With those changes, an error is popped-up as the extension was loading:

image

but the extension still activated and running any harness leads causes the wheel to keep spinning forever.

Copy link
Contributor

@jaisnan jaisnan left a comment

Choose a reason for hiding this comment

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

Thanks!

@zhassan-aws zhassan-aws merged commit 26e29b7 into model-checking:main May 17, 2023
@zhassan-aws zhassan-aws deleted the iss72 branch May 17, 2023 17:44
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.

Issue friendly error if the extension cannot execute Kani
2 participants