- Issue created by @traviscarden
- π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
IMHO functional tests (
BrowserTestBase
+ Cypress E2E tests) is sufficient for now β that's sufficient for testing the common paths.The goal for OpenAPI is to help the front and back ends to stay in sync with more ease, not for that to become a goal unto itself.
IOW:
- OpenAPI to ensure valid request & response shapes, running only during development
- actual validation of requests when those requests mutate server-side state β i.e. this is what
src/Plugin/Validation/Constraint/*
will be for.
That being said, if you see a low-effort/maintenance, high-impact/reach way to write kernel or unit tests to verify certain edge cases in request/response bodies are caught automatically by OpenAPI, then I'd definitely welcome that :) But that should not be a multi-week/month endeavor IMHO.
- Merge request !284Add a test for the OpenAPI spec for `/api/layout/{entityTypeId}/{entityId}`. β (Open) created by traviscarden
- Assigned to wim leers
- Status changed to Needs review
3 months ago 1:13am 10 September 2024 - πΊπΈUnited States traviscarden
tl;dr: I agree with everything you said, and I have an "inexpensive" solution I would like to implement that I would consider to satisfy the requirements of this issue.
IMHO functional tests (
BrowserTestBase
+ Cypress E2E tests) is sufficient for now β that's sufficient for testing the common paths.I agree.
The goal for OpenAPI is to help the front and back ends to stay in sync with more ease, not for that to become a goal unto itself.
Yes. We're on the same page here.
That being said, if you see a low-effort/maintenance, high-impact/reach way to write kernel or unit tests to verify certain edge cases in request/response bodies are caught automatically by OpenAPI, then I'd definitely welcome that :)
I have proposed one in the above MR. I actually hadn't planned on looking at this issue at all yet, but working on π Add missing API paths + request bodies to `openapi.yml` Active I was having a hard time testing my assumptions about how the specification itself was being interpreted. And the ability to just whip up a quick PHP array and get instant feedback really clarifies things and saves a lot of "throwing things at the wall" and "testing by coincidence". (Turns out even YAML can be written test-first! π)
But that should not be a multi-week/month endeavor IMHO.
It just took a day. π
Re: the MR
The tests are passing, but I notice PHPUnit reports two tests skipped--which I assume are the two that this MR adds. That is doubtless because the tests depend on
league/openapi-psr7-validator
, which I assume isn't installed on CI. Ordinarily that would constitute a problem that needs solving, of course, but I'm actually fine with it in this case, since its whole point is to aid in rapid development and the real "regression tests" are in Cypress. Developers can just run them ad hoc in PhpStorm, for example (as I've been doing), when actually modifyingopenapi.yml
and ignore them the rest of the time. They could technically get out of sync, but as far as I'm concerned, we can create a follow-up to worry about that later.So I would like to propose the MR above as a rapid development tool that satisfies our minimal requirements and lays a foundation of future iteration if that ever seems valuable. According to your judgment, we can create a follow-up issue and/or a
@todo
or a note in theREADME
I added to capture the above issue of the tests not getting run on CI. - Assigned to traviscarden
- Status changed to Needs work
3 months ago 10:11am 10 September 2024 - π§πͺBelgium wim leers Ghent π§πͺπͺπΊ
On the right track π β¦ but it's too PHP-heavy. This test coverage must have a very low bar for non-PHP developers. See MR with suggestions.
- πΊπΈUnited States traviscarden
FYI: I'm setting this issue aside for the time being in favor of higher priorities.