OSC / ood_core

Open OnDemand core library
https://osc.github.io/ood_core/
MIT License
9 stars 26 forks source link

refactor tests to minitest #844

Open johrstrom opened 1 month ago

johrstrom commented 1 month ago

While attempting to write a test case for a feature, I renewed my favor for minitest over rspec.

I just think rspec is too hard to learn and deal with when we have all sorts of other stuff going on.

So this epic ticket is to refactor all of our test suite from rspec to minitest to help promote writing test cases without having to learn rspec. Minitest has this advantage - you don't really have to learn minitest. It's just Ruby with some helpers, whereas rspec has it's own DSL that you have to learn and troubleshoot.

johrstrom commented 1 month ago

@treydock I know you prefer rspec, I just think minitest is easier all around. Especially as the team grows and attempts to attract new developers, not to mention current developers' appetite to have to all these various frameworks.

I guess really - minitest just helps us narrow all the things one may have to learn/know to get anything done.

treydock commented 1 month ago

With simplicity comes limitations, like with most things. RSpec makes it easy to do complex stubbing (mocking) as well as support numerous kinds of matchers that read well. Also keeping tests from spilling into other tests is a huge benefit to RSpec.

Before you fully commit to replacing RSpec, might be good to take one of two complex test cases from each RSpec file and make sure you can adequately replace them with minitest. Simplicity at the cost of functionality is not a good trade off. If minitest is going to make testing less reliable or function in a way that makes them less useful, that seems like a deal breaker.

Also the e2e tests in main OnDemand repo have to stay with Rspec cause there is simply no way to use minitest to replace beaker-rspec plugin.

johrstrom commented 1 month ago

Before you fully commit to replacing RSpec, might be good to take one of two complex test cases from each RSpec file and make sure you can adequately replace them with minitest

Yea I'm really just trying to reduce yak shaving where I can. I'll take a look at some complex test cases and see how they fit.

Also the e2e tests in main OnDemand repo have to stay with Rspec cause there is simply no way to use minitest to replace beaker-rspec plugin.

This effort is really limited to this library, there's nothing we can do about e2e tests.

johrstrom commented 1 month ago

Before you fully commit to replacing RSpec, might be good to take one of two complex test cases from each RSpec file and make sure you can adequately replace them with minitest

Yea I'm really just trying to reduce yak shaving where I can. I'll take a look at some complex test cases and see how they fit.

I've got a pull request going for this in #846 where I'll add more comments there. Turns out we need the mocha gem to stub things correctly. Then some things we need to write ourselves like testing method interfaces.