Conversation
c9c2fda to
ccc5022
Compare
|
Thanks @jdesrosiers , this is really helpful! I notice that we also now have #3511 and also that since we've been upgrading other dependencies, there's a conflict in |
ccc5022 to
a7ff760
Compare
|
@lornajane Done. Rebased and conflicts fixed. |
lornajane
left a comment
There was a problem hiding this comment.
Thanks! Tests run well, I'm happy to see the update.
|
@handrews This pull request overlaps with the other open dependency updates, I suggest we give priority to the human-driven changes (it also includes one update and eliminates another) |
|
This isn't my area of expertise, but given that the tests are running (thanks for confirming, Lorna!), I think questions would be around what other implications there are around changing the testing framework.
|
|
@earth2marsh The primary purpose of this PR was to update the
Yes, that's fair. Tests are contributed by adding OpenAPI documents to the pass/fail directories, so most contributors won't need to work with this code directly. The tests can be run using the same
That's fair. Vitest is pretty new. It's built by the people who maintain vite, which is a well established core component of the JavaScript ecosystem. Given the strength of the community behind vite, I don't think vitest is going to disappear anytime soon. But, even if it did, it's not a big deal. The code here is small and that shouldn't change. If this test suite gets used, it will grow by adding OpenAPI documents to the pass/fail directories, not by adding code. It's so little code that it's trivial to change to a different framework for whatever reason. It literally took about a minute to switch to vitest and changing to something else in the future would likely be similarly trivial.
I checked to make sure the framework wasn't being used anywhere else. Changing the framework should not effect anything other than these tests. |
|
Thank you for confirming--it helped me better understand the reasoning behind the change and its potential blast radius. Even more importantly, thanks for the contribution. 🙏 |
I noticed #3503 in my notifications, so I figured I'd do the upgrade real quick. While I was at it, I moved to a nicer test framework.
Honestly, I don't think anyone even remembers these tests exist and they certainly don't ever get run or added to when changes are made to the schema like they should. If this isn't used, it's probably better to just remove it all together. Anyway, this is the upgrade should you choose to keep it around.