Skip to content

Move cargo tests to compiletest.#198

Merged
danielsn merged 5 commits intomodel-checking:main-152-2021-06-07from
bdalrhm:cargo
Jun 16, 2021
Merged

Move cargo tests to compiletest.#198
danielsn merged 5 commits intomodel-checking:main-152-2021-06-07from
bdalrhm:cargo

Conversation

@bdalrhm
Copy link
Copy Markdown
Contributor

@bdalrhm bdalrhm commented Jun 11, 2021

Description of changes:

This PR moves cargo tests to compiletest. expected.* files are renamed to *.expected to make use of available convenience functions. Note that a --target-dir option is added to change the default target directories for cargo builds to build/<arch>/test/cargo/<proj-name>/<function-name>/target and avoid race problems while running cargo tests concurrently. For consistency, other generated files are also moved into the specified target directory.

Resolved issues:

Resolves #177

Call-outs:

Testing:

  • How is this change tested?
    • Cargo tests still pass after the changes
  • Is this a refactor change?
    • To some degree, cargo-rmc-tests is moved and renamed to src/test/cargo

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.

@bdalrhm bdalrhm self-assigned this Jun 11, 2021
@bdalrhm bdalrhm requested review from adpaco-aws and vecchiot-aws and removed request for vecchiot-aws June 11, 2021 21:42
Copy link
Copy Markdown
Contributor

@adpaco-aws adpaco-aws left a comment

Choose a reason for hiding this comment

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

This looks great! Could we use CargoRMC and cargo-rmc for the enum variant and its associated string?

@adpaco-aws adpaco-aws requested a review from vecchiot-aws June 13, 2021 08:46
parser.add_argument("--mangler", default="v0")
parser.add_argument("--visualize", action="store_true")
parser.add_argument("--srcdir", default=".")
parser.add_argument("--target-dir", default="target",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know how to comment on a non-changed file, but should we be adding this argument to scripts/rmc as well?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought about that, but I could not come to a conclusion. Normally, when someone runs cargo, they expect all artifacts (besides Cargo.lock) to be in the target directory. rustc has a somewhat similar option --out-dir for specifying the directory of the compiled file. However, it's not really treated as a build directory and I don't know if we should treat it as such. So, I think this is a decision for the team to make.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In addition to @bdalrhm explanation, I would like to point out that RMC's workflow is more similar to CBMC's: It does generate a verification output, not a compiled program. So it makes sense for us to have this --target-dir flag for cargo-rmc but not for rmc itself.

@danielsn danielsn merged commit a4d575a into model-checking:main-152-2021-06-07 Jun 16, 2021
adpaco-aws added a commit that referenced this pull request Jun 17, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Jun 23, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Jul 2, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Jul 9, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Jul 15, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Jul 26, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Aug 2, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
@zhassan-aws zhassan-aws mentioned this pull request Aug 6, 2021
4 tasks
adpaco-aws added a commit that referenced this pull request Aug 6, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Aug 17, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
adpaco-aws added a commit that referenced this pull request Aug 24, 2021
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
tedinski pushed a commit to tedinski/rmc that referenced this pull request Apr 26, 2022
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
tedinski pushed a commit that referenced this pull request Apr 27, 2022
* Move cargo tests to compiletest.

* Rename `cargo` to `cargo-rmc`.

* Add an expected line to simple-main test.

Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
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.

Port Cargo Tests into compiletest.

4 participants