-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Conversation
Generated by 🚫 dangerJS |
Made good progress today, just a bit slow going managing the default data stuff :/ Anyways, this tests a lot of stuff and helped me find some bugs in the app around how we show actions in the thread settings dropdown. While there are still more thread tests to write, let's go ahead and ship this since it includes the fix + adds a lot of coverage regardless :) |
@mxstbr I was having a lot of trouble getting eslint to run locally; I added |
These tests are a nightmare - i cannot figure out how to reliably reset the db between specs because it just fails and tries to start running other tests while the db is still resetting. And if I don't reset, we have collisions in the data when testing migrations. So we need to figure out how to reliably reset the db or just never test migrations in these end to end tests and only test that certain view elements load...and eventually figure out mutation tests? |
This test will likely fail because of this, but each works individually. Let's ship |
Uhh, doesn't that make these tests totally useless? Doesn't seem to worth to ship this, right? |
Agreed, I'll keep working on this today. Needed to get this out as it fixed a bunch of bugs + improved a ton of coverage on thread views. |
Makes sense. I'm trying to fix 'em in CI at #2674 |
Status
I'm going to be working on this for a while more today, just wanted to get it on github :)