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

Proposal to rework forc test command to only perform embedded, in-language, unit testing. #1833

Closed
mitchmindtree opened this issue Jun 3, 2022 · 2 comments
Assignees
Labels
big this task is hard and will take a while blocked compiler General compiler. Should eventually become more specific as the issue is triaged forc testing General testing

Comments

@mitchmindtree
Copy link
Contributor

Currently, forc test is a small wrapper around forc build and cargo test.

Its current behaviour can be summarised as "a runner for Rust application integration tests".

Proposal

Change the behaviour of forc test to only support in-language unit testing, and leave application integration tests to the application level.

Motivations

Developers building pure Javascript/Typescript applications should not need to know about Rust.

Currently, Sway testing is introduced to JS/TS devs through the forc test command. They need to install the toolchain for Rust and learn Rust just to run some Rust SDK based integration tests that don't necessarily even map to the integration that's going on in their actual JS/TS application. They likely need to run these tests as well as their existing JS/TS application test suite anyway.

At the moment, JS/TS application devs are arguably better off deleting the harness.rs, ignoring that forc test exists and writing their own integration testing using the typescript SDK in a manner that better integrates with how their existing testing and application is structured.

Confusion and conflicts when attempting to test projects in an existing Rust workspace.

forc test is a very small wrapper around cargo test. This indirection causes developers that are having (potentially unrelated) trouble with their testing to confuse errors as if they are a problem with forc test, when they'd likely be much easier to debug with the understanding that they are just invoking cargo test.

The generated test template and its location within the forc project directory also implies that the test must be within some subdirectory of the forc project. The reality is that because we're relying on cargo, it is the workspace Cargo.toml that is considered the project root. This leads to issues where:

  • Devs think that they must keep their integration tests within subdirectories of their respective Sway projects, when instead they could be included with the rest of their integration testing.
  • Issues occur where running forc test within a Sway project that does not yet have a cargo manifest. When a user does this, cargo assumes that the user is referring to the enclosing workspace and will suddenly start running all tests in the workspace. This could potentially be mitigated by checking for and parsing the test harness's associated cargo manifest for the test package name and passing that name to a --package <name> flag to cargo test, but this just works to maintain the illusion that forc test is special when it is not.

Pollutes the Sway project workspace with cargo manifest, Rust harness.rs, target directories.

Related to the above two points. Currently, we have to make some assumptions about the user's Sway application development environment in order to make it possible for them to run tests. This means we need to bea able to provide them with the basic cargo building blocks including the cargo manifest and a harness file. Cargo itself will also generate a target directory for its compiler output artifacts. This may be in the current directory, but might also be at the workspace level if the forc project happened to be instantiated within a cargo workspace.

forc projects would feel a lot cleaner / more consistent if we didn't need the doubling of manifests, Rust source and compiler output artifact directories.

Summary

forc test needlessly imposes the Rust language and toolchain on developers. It does so in a way that potentially causes confusion/conflicts if performed within an existing Rust workspace. forc test itself provides no benefit over regular Rust application testing, despite the dedicated command implying something special is happening. It needlessly adds noise to forc project directories.

Proposed Solution

  1. Rework forc test behaviour to have nothing to do with Rust integration testing. Instead, forc test purely runs in-language Sway unit tests. This means that we can guarantee that forc will provide everything out of the box necessary to run its forc test command. This separates the concerns of Sway unit testing from application integration testing, avoiding the need of imposing any assumptions about the user's application development environment. This is of course the bigger, blocking step, but the recent introduction of attributes provides a path forward:

  2. Provide a forc-test library crate for use with Rust integration testing. This would simply re-export and extend the existing fuels API with short-hand functions for easily pre-building forc projects so that ABI and bytecode are available to the integration tests. This should live in the Sway repo alongside forc and not in the SDK repo. This keeps the separation of concerns between forc and the fuel SDK clear, while still providing high-level forc functionality to Rust application developers who want to test Sway integration.

  3. Provide a cargo generate template for Rust integration tests. This would allow Rust devs who want to test Sway integration in their Rust applications to run a single command to initialise a Rust project ready to start testing. This would replace the need for forc init to create a Rust test harness, cargo manifest and so on, and give the Rust developer more flexibility over where they want to do their Rust application integration testing (i.e. not necessarily in the middle of one of their forc projects).

  4. Update the documentation to remove assumptions about requiring Rust for testing. Instead, add a section on how to test Sway integration with Rust applications that clearly describes how to use the aforementioned forc-test crate within cargo tests, and how to use the cargo generate template to quickly setup an integration testing project.

  5. Consider providing steps 2, 3 and 4 for typescript developers. That is, a forc-test typescript library, a way to generate a typescript test harness, and documentation on how to do typescript integration testing in the Sway book.

@mitchmindtree mitchmindtree added compiler General compiler. Should eventually become more specific as the issue is triaged forc big this task is hard and will take a while testing General testing blocked labels Jun 3, 2022
@otrho
Copy link
Contributor

otrho commented Jun 3, 2022

Nice. The first thing I do after forc init these days is delete Cargo.toml and tests/harness.rs...

mitchmindtree added a commit that referenced this issue Jul 7, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
@mitchmindtree mitchmindtree self-assigned this Jul 20, 2022
mitchmindtree added a commit that referenced this issue Jul 22, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
mitchmindtree added a commit that referenced this issue Jul 22, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
mitchmindtree added a commit that referenced this issue Aug 21, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
mitchmindtree added a commit that referenced this issue Sep 19, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
mitchmindtree added a commit that referenced this issue Oct 3, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
mitchmindtree added a commit that referenced this issue Oct 6, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
mitchmindtree added a commit that referenced this issue Oct 9, 2022
As proposed in #1833, this
removes the integration testing implementation under `forc test` in
anticipation of using the command for unit testing.
mitchmindtree added a commit that referenced this issue Oct 10, 2022
…ation of unit testing support (#2264)

This PR is a WIP that aims to address the first 3 steps in #1833.

This is in anticipation of using `forc test` to support unit testing
(rather than Rust integration testing) #1832.

The `forc test` command remains for now, but outputs a message
explaining that the command is now reserved for unit testing and links
to the issues above.

## TODO

- [x] Create a new `sway-test-rs` repo or similar that can be used as a
`cargo generate` template.
- [x] Update Rust integration testing docs in the Sway book to describe
how to use the `cargo generate` command to easily add Sway integration
testing to an existing Rust project.

## Follow-up

- Create a `forc-test-rs` crate that re-exports and extends `fuels` with
useful `forc` functionality for integration testing (e.g. re-exporting
`forc_pkg::build` to ensure sway code is built and available under
`out/` at the start of testing).
@mitchmindtree
Copy link
Contributor Author

Closed via #2264. Follow #1832 for progress on in-language unit testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big this task is hard and will take a while blocked compiler General compiler. Should eventually become more specific as the issue is triaged forc testing General testing
Projects
None yet
Development

No branches or pull requests

2 participants