cucapra / filament

Fearless hardware design
http://filamenthdl.com/
MIT License
160 stars 9 forks source link

running `cargo test` from the main directory fails #447

Closed CrepeGoat closed 2 months ago

CrepeGoat commented 2 months ago

system

setup

as described in the docs:

$ git clone --depth=1 git@github.com:cucapra/filament.git
$ cd filament
$ cargo build
(... this works)

errors

$ cargo test
...
running 3 tests
test crates/ast/src/control.rs - control::Bundle (line 360) ... FAILED
test crates/ast/src/control.rs - control::BundleType (line 299) ... FAILED
test crates/ast/src/control.rs - control::ForLoop (line 247) ... FAILED

failures:

---- crates/ast/src/control.rs - control::Bundle (line 360) stdout ----
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `f`
 --> crates/ast/src/control.rs:361:8
  |
3 | bundle f[10]: for<i> ['G+i, 'G+i+1] W;
  |        ^ expected one of 8 possible tokens

error: aborting due to 1 previous error

Couldn't compile the test.
---- crates/ast/src/control.rs - control::BundleType (line 299) stdout ----
error: expected one of `async`, `move`, `static`, `|`, or `||`, found `[`
 --> crates/ast/src/control.rs:300:8
  |
3 | for<i> ['G+i, 'G+i+1] W
  |        ^ expected one of `async`, `move`, `static`, `|`, or `||`

error[E0658]: `for<...>` binders for closures are experimental
 --> crates/ast/src/control.rs:300:1
  |
3 | for<i> ['G+i, 'G+i+1] W
  | ^^^^^^
  |
  = note: see issue #97362 <https://github.com/rust-lang/rust/issues/97362> for more information
  = help: consider removing `for<...>`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0658`.
Couldn't compile the test.
---- crates/ast/src/control.rs - control::ForLoop (line 247) stdout ----
error: unexpected token: `...`
 --> crates/ast/src/control.rs:248:17
  |
3 | for i in 0..W { ... }
  |                 ^^^
  |
help: use `..` for an exclusive range
  |
3 | for i in 0..W { .. }
  |                 ~~
help: or `..=` for an inclusive range
  |
3 | for i in 0..W { ..= }
  |                 ~~~

error[E0586]: inclusive range with no end
 --> crates/ast/src/control.rs:248:17
  |
3 | for i in 0..W { ... }
  |                 ^^^ help: use `..` instead
  |
  = note: inclusive ranges must be bounded at the end (`..=b` or `a..=b`)

error[E0425]: cannot find value `W` in this scope
 --> crates/ast/src/control.rs:248:13
  |
3 | for i in 0..W { ... }
  |             ^ not found in this scope
  |
help: you might have meant to write `.` instead of `..`
  |
3 - for i in 0..W { ... }
3 + for i in 0.W { ... }
  |

error: aborting due to 3 previous errors

Some errors have detailed explanations: E0425, E0586.
For more information about an error, try `rustc --explain E0425`.
Couldn't compile the test.

failures:
    crates/ast/src/control.rs - control::Bundle (line 360)
    crates/ast/src/control.rs - control::BundleType (line 299)
    crates/ast/src/control.rs - control::ForLoop (line 247)

test result: FAILED. 0 passed; 3 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.14s

error: doctest failed, to rerun pass `-p fil-ast --doc`
rachitnigam commented 2 months ago

We do not have any inline tests in this repo so cargo test is expected to not work.

CrepeGoat commented 2 months ago

if there are no inline tests, shouldn't cargo test collect zero tests and pass? not sure I understand the project setup here.

I'm asking because I'm trying to write a nix derivation to build and install filament (in my case, specifically on macOS, but this would be applicable to linux builds as well). it just so happens that the de-facto rust project builder on nix, rustBuildPackage, runs cargo test after building to check that the build was successful, and in the case of this repo, that fails. I can likely turn off that check somehow, but the build should ideally have some validity check, and it's not clear to me what that would be. (as I commented in #301, the docs instructions for checking for a correct compiler build seem to be outdated.)

please advise, thanks

rachitnigam commented 2 months ago

Okay, got it! The intension is definitely that we don't have any inline tests so if we are triggering them, that is a bug and should be fixed.

Do you want to take a shot at fixing this? Also, thanks for building a nix configuration. We'd love to add it directly to the repo and provide it as a possible build option for new users.

CrepeGoat commented 2 months ago

I think my employment contract would give my company ownership of anything I directly contribute, so unfortunately I think it's not really feasible for me to contribute, neither for fixing the errors nor posting a nix build 😢 otherwise I'd absolutely be interested. sorry

UnsignedByte commented 2 months ago

The issue is that rust had built in documentation tests for code blocks in doc comments. This is easily fixed by setting the doc comment language to fil instead of nothing, which would default to rust.

CrepeGoat commented 2 months ago

I just pulled main and reran cargo test; it looks like #448 fixed 2 out of the 3 failures.

-> can this be reopened until the 3rd is fixed?