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

Broken tests in html-macro 0.7.0 and 0.7.1 #170

Open
dbrgn opened this issue Feb 14, 2022 · 4 comments
Open

Broken tests in html-macro 0.7.0 and 0.7.1 #170

dbrgn opened this issue Feb 14, 2022 · 4 comments
Labels
bug Something isn't working

Comments

@dbrgn
Copy link
Collaborator

dbrgn commented Feb 14, 2022

I upgraded to html-macro 0.7.1, but my project testsuite started failing due to the way spaces are handled.

So as a sanity-check, I ran the tests in percy itself:

$ cd crates/html-macro-test
$ cargo test
failures:
    tests::all_tests::custom_component_children
    tests::all_tests::custom_component_props
    tests::all_tests::punctuation_token
    tests::all_tests::sibling_text_nodes
    tests::all_tests::text_next_to_block
    tests::text::text_multiple_text_space_around
    tests::text::text_multiple_text_space_between
    tests::text::text_multiple_text_space_between_around
    tests::text::text_no_space_before_open_tag
    tests::text::text_root_node
    tests::text::text_space_after_block
    tests::text::text_space_after_start_tag
    tests::text::text_space_before_block
    tests::text::text_space_before_end_tag
    tests::text::text_space_before_next_open_tag
    tests::text::text_tokens_in_between_vars_space_around_between
    tests::text::text_tokens_in_between_vars_with_space
    tests::ui::ui

test result: FAILED. 33 passed; 18 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.27s

It seems like a lot of tests are failing. This happens on commit 2aed654 (0.7.0) as well.

@dbrgn dbrgn added the bug Something isn't working label Feb 14, 2022
@chinedufn
Copy link
Owner

cargo -V?
I'm trying to see if you're on stable or nightly.

Right now text works slightly differently on stable vs. nightly.

https://github.com/chinedufn/percy/blob/2aed65476bc4d4660c0eb2fe6c733ce96bf6aee7/README.md#stable-rust

One approach would be to add a nightly feature flag to html-macro that controls whether or not unquoted text is supported. So that people can't accidentally use unquoted text on stable.

@dbrgn
Copy link
Collaborator Author

dbrgn commented Feb 17, 2022

https://github.com/chinedufn/percy/blob/2aed65476bc4d4660c0eb2fe6c733ce96bf6aee7/README.md#stable-rust

Aah, that explains it! I previously used nightly, but migrated to stable recently. The tests pass on nightly (except for one test that does assertions with regards to error message output.)

One approach would be to add a nightly feature flag to html-macro that controls whether or not unquoted text is supported. So that people can't accidentally use unquoted text on stable.

That would be nice. Or a unquoted-text feature that throws a compile-time error if used on a non-nightly Rust version.

@dbrgn
Copy link
Collaborator Author

dbrgn commented Feb 17, 2022

The quoting workaround is a bit awkward due to double curly braces when using top-level text:

html! { { "Hello World" } },

However, I guess we'll have to live with this limitation for now.

@chinedufn
Copy link
Owner

There shouldn't be a fundamental reason that we couldn't support html!{ "Hello World" }, would probably just require some tweaks to the parsing logic.


I'm also realizing that we shouldn't need a feature flag.

If you aren't on nightly then Span.line() and Span.column() always return 0, so whenever we encounter unquoted text we can just check the line/column and if they are both zero we create a compile time error that unquoted text only works on nightly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants