Add a simple test.

[tompscanlan]

Adding this to open discussion around the issue #7 , and to lay framework for fixing #97

[tbillington]

I like the direction of this! Thank you for having a crack.

A few points of feedback :)

What you're doing would make more sense to me as an integration test of kondo_lib. So it would go in kondo_lib/tests and probably call scan. Needing to go through discover from kondo is quite awkward.

I'd also suggest test utilities to create the test projects as opposed to having them hardcoded in the repo, like you've done in _create_fake_python_project. You can see a basic example of this in use by the homebrew install code.

Avoid referencing a absolute path that is based on your home directory.

[tompscanlan]

Thanks, I use tests to help understand what's going on, and it grew from main, so I ended up testing from the wrong place.

I'm really trying to isolate parts, so I'll stick to the unit test model, but relocate to the right module.

I'll iterate and ping you next time.

[tompscanlan]

ok... done with changes for a few hours @tbillington. Thoughts?

[tbillington]

This is looking good so far, appreciate you iterating on it. Adding tests for the first time is always a bit painful :)

There's really only a few blockers for me

• As I mentioned in the test.rs comment, specifying the test directory structure in code is strongly preferred
• I hadn't realised lints weren't applying to tests, I've updated the CI job so please rebase/merge with master to get that (and some example tests in kondo_lib)
• Moving the tests to kondo_lib is strongly preferred to avoid changing the public api, let me know if you'd like more specific info on this

I'd also suggest you enable clippy in your editor of choice if possible, it's a great rust learning tool! If you're using vscode you just need to add "rust-analyzer.check.command": "clippy", to your settings.json.

If you have any questions don't hesitate to ask

[tompscanlan]

Hey @tbillington,

Thanks for the feedback. I'm still really new to rust, so I haven't used clippy. I'll look into it. Thanks.

re: Specify test in kondo-lib I'm intending to test discover and clean of kondo, not kondo-lib. Since those live in kondo, I'm planning on keeping tests in kondo. I can add some tests for kondo-lib. I'm feeling the resistance here, but I'm pretty keen on keeping these. Want to hop in files review or a quick screen share session to mark up parts that really bug you?

re linting, I'll refresh today (I'm US east coast, so we're dominating the 24 hour clock).

re changing public api. I'm not following where I'm changing the public api. Can you point at something that changed to a user? Splitting kondo/main.rs into kondo/lib.rs and main.rs could be doing something I'm not aware of.

I'll look closer at kondo-lib today and see if what you say sinks in. Will make another iteration and see your feedback tomorrow.

[tompscanlan]

Ok... think I'm following on where I was testing from kondo into the lib for clean(). I moved that kondo-lib, but still seeing need for the discover() test to remain in kondo.

Peek at this revision and let me know.

[tompscanlan]

Review what's here at your leisure once more and I'll make another pass before rebasing again.

A part I'd like to change. but I'm not sure if I'll get to today... is to move the programmatically created project into the unit test, and the file-based test out to the integ/cli test. Including your wrapper syntax around temp dirs.

[tompscanlan]

I think I got to almost all your feedback. I like the wrapper idea and played with it. Ready for feedback.

[tbillington]

Sorry I haven't gotten to this yet, been a bit busy.

You mentioned being open to chat, just curious which timezone you're in? I'm in GMT+10

[tbillington]

Re-formatting, I have this in my settings.json

  "[rust]": {
    "editor.defaultFormatter": "rust-lang.rust-analyzer",
    "editor.formatOnSave": true
  },

[tompscanlan]

I'm GMT-4. My night/your morning to midday are the best opportunities. I'm also a bit tied up, but I'll get to this this week.

[tompscanlan]

This should pass the ci, at least :)

Open questions I have:

1. what to do about duplicated unit test code for creating the fake project. Ignoring it is an option. I think pushing it into a test module and calling that from both tests might work.

2. docs? Want anything in the readme?

3. I've got a couple test cases that were failing so I "ignored" them. I might be mistaken or misunderstanding, so take those with a grain of salt.

4. re chatting. I can be open around 930pm GMT-4 some time. Do you have a favorite coms channel to use?

[tbillington]

Sorry for the slow reply on this Tom, had a few things on my plate but it's on my list to get around to this hopefully this week.

[tompscanlan]

Not a problem. PRs can wait, take care of what you need.

[tompscanlan]

adding a few tests around path_canonicalize(). Seems like there is a bug there when --ignored-dirs contains dirs that don't exist, and aren't absolute.