Write out FixturesSpecHelper

[bagedevimo]

Having this helper generate these tests in before the individual test
runs means they are not included any the standard rspec transactional
tests mechanism. This means that if the suite cannot fully complete for
any reason, such as a syntax error, or if the number of create/destroy
is not perfectly matched in a test then the following test run will
fail.

Using let(:x) and before(:each) is best practise, and shouldn't have a
significant effect on test runtimes.

[bagedevimo]

@tom93 keen to know if you any objections to this idea, before I continue through and do the rest of the tests :)

[tom93]

I'm not that familiar with rspec, but having the second test run be independent sounds like a pretty good idea!
It would be good to check the performance impact.

Small clarifications:

Having this helper generate these tests in before the individual test

Do you mean generate these fixtures/models?

Using let(:x) and before(:each)

I'm a little confused, it seems like this PR is removing uses of before(:each).

[tom93]

Using let(:x) and before(:each)

I'm a little confused, it seems like this PR is removing uses of before(:each).

Do you mean that before(:all) is bad, and let/before(:each) are good? (I see that a bare "before" is the same as before(:each), and "let" seems like a lazy version of before(:each) so basically interchangeable.)

[Holmes98]

Do you mean that before(:all) is bad, and let/before(:each) are good?

Yeah, I think that's the intention (was discussed briefly on Discord: https://discord.com/channels/670126531489824788/678154623571329025/1363084851737137272)

[bagedevimo]

Yes, clearly I needed to read my commit message better. let(:x) is a preferred mechanism to assigning an ivar in a before block, (https://www.betterspecs.org/#let). One big reason its better is that it has much clearer / more obvious declarations, especially when you use nested contexts.

Yes, before do and before(:each) do are the same thing. Just a more common way of writing it.

Having this helper generate these tests in before the individual test

Yes, this was supposed to read models. Will update soon.

[bagedevimo]

I have updated the commit message, but I haven't copied it back into the GH PR description yet.

[bagedevimo]

• Upgrade gemfile for Rails 5.0.7.2 #298 : 2 dependent PRs (#314 , #315 )
• Configure the hybrid cookie session store #302
• Write out StrongPresenter #297
• Add basic feature test for problem browsing #300
• Remove the squeel gem #296
• Write out FixturesSpecHelper #290 👈 (View in Graphite)
• Remove most XML responses #292
• Refactor db/seeds to allow for easy development seeds #293
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[coveralls]

coverage: 37.513% (-0.2%) from 37.743%
when pulling b48f347 on write-out-fixtures-spec-helper
into 0e0fec6 on master.

[Holmes98]

This is around 10 seconds slower for me (23s vs 13s), but I'm okay with that.

[bagedevimo]

This is around 10 seconds slower for me (23s vs 13s), but I'm okay with that.

Huh I might get you to run a profiler at some point as I don't get that change - its within 1s and within the stddev for me.

[Holmes98]

It's also around 10s slower in CI (in the rspec step):
https://github.com/NZOI/nztrain/actions/runs/15096264112/job/42431328829
https://github.com/NZOI/nztrain/actions/runs/15096266863/job/42431334619

[bagedevimo]

Hmm you are right - I will dig into that later. I don't think its a blocker, and I am sure I can make it faster again.

[Holmes98]

Yeah I'm fine with just merging this for now.

[tom93]

I'd like to take a look!

[tom93]

I think all the new time is spent hashing passwords, which should be should be possible to fix (using caching). I'll look into it.

[bagedevimo]

Thanks Tom, appreciate you! It could be as simple as we hard-code a passwordhash rather than re-hashing, which I think is a relatively common solution.

[tom93]

Unfortunately Devise's "password=" method (in lib/devise/models/database_authenticatable.rb) unconditionally recomputes the hash (even if we manually set the encrypted_password attribute). This <https://blog.plataformatec.com.br/2011/12/three-tips-to-improve-the-performance-of-your-test-suite/> suggests setting "Devise.stretches = 1" in the spec helper, other ideas welcome!

[Holmes98]

Nice, can confirm that setting Devise.stretches = 1 fixes it.

Could maybe set that conditionally in the initializer instead of spec_helper, since that's what the Devise currently does in the template:
https://github.com/heartcombo/devise/blob/fec67f98f26fcd9a79072e4581b1bd40d0c7fa1d/lib/generators/templates/devise.rb#L126

But either way works.