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

add cargo bless functionalty #1091

Open
lucarlig opened this issue Feb 26, 2024 · 16 comments
Open

add cargo bless functionalty #1091

lucarlig opened this issue Feb 26, 2024 · 16 comments

Comments

@lucarlig
Copy link

I have been using dylint quite a lot, the cargo bless functionality would save so much repetitive work copying and pasting.

@lucarlig
Copy link
Author

maker seems to make it work with a wrapper around uitests.

@smoelius
Copy link
Collaborator

Thank you very much for your feedback. I will look into this.

@lucarlig
Copy link
Author

lucarlig commented Feb 28, 2024

also a better diff when running the test would be so much appreciated I opened this Manishearth/compiletest-rs#284 but @Manishearth mentioned that the crate is not actively maintained so no luck with this.
Let me know if you make a plan to improve UI testing for dylint and how I can help.

@smoelius
Copy link
Collaborator

Do you know if migrating dylint_testing from compiletest to ui_test would allow for better diffs?

@lucarlig
Copy link
Author

doesn't seem to allow it, maybe could be added but the default one is better than compiletest already. I think rust really needs a better UI test framework...

@lucarlig
Copy link
Author

oli-obk/ui_test#205

@smoelius
Copy link
Collaborator

Generally speaking, do you know how difficult it is for a package to switch from using compiletest to ui_test?

@lucarlig
Copy link
Author

no idea, but I can give it a try.

@smoelius
Copy link
Collaborator

That would help me out a lot!

@lucarlig
Copy link
Author

will try this next week @smoelius

@smoelius
Copy link
Collaborator

Thanks!

@lucarlig
Copy link
Author

lucarlig commented Mar 13, 2024

i tried something like this

    let mut rustc_flags: Vec<OsString> = config.rustc_flags.iter().map(OsString::from).collect();
    rustc_flags.push(OsString::from("--emit=metadata"));
    if cfg!(feature = "deny_warnings") {
        rustc_flags.push(OsString::from("-Dwarnings"));
    }
    rustc_flags.push(OsString::from("-Zui-testing"));

    let program = ui_test::CommandBuilder {
        args: rustc_flags,
        ..ui_test::CommandBuilder::cmd(driver.to_path_buf())
    };
    let config = ui_test::Config {
        program,
        ..ui_test::Config::rustc(src_base.to_path_buf())
    };
    ui_test::run_tests(config).unwrap();

instead of compile_tests, issue is the args order doesn't work with driver, it's expecting the file path to be first.

@smoelius
Copy link
Collaborator

Thank you very much for looking into this.

instead of compile_tests, issue is the args order doesn't work with driver, it's expecting the file path to be first.

I'm not quite following. Could you post the error message?

@lucarlig
Copy link
Author

lucarlig commented Mar 13, 2024

this is how it's passing the command:

command: "~/.dylint_drivers/nightly-2024-03-02-aarch64-apple-darwin/dylint-driver" "--emit=metadata" "-Zui-testing" "/private/var/folders/qb/g3v983vx5px1qx4bhcwqx3700000gn/T/.tmpbiKj0k/ui/main.rs" "--edition" "2021"

the issue is dylint-driver seems to expect as first arg the filename instead is getting "--emit=metadata"
ui_tests always set the filename as last

@smoelius
Copy link
Collaborator

Is it worth opening a WIP PR for what you have done?

@lucarlig
Copy link
Author

probably not, it is just literally that code I posted above.

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

No branches or pull requests

2 participants