You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
With the ContentExample class in test/model/content_test.dart, we have a number of different examples of Zulip message content. These come in the form of HTML, because that's what the app consumes.
It's important that for most of these tests, the HTML is something a Zulip server actually produced, because the job of these tests is to make sure we're successfully parsing whatever the server sends us, with all its quirks and sometimes messes. (Parsing and displaying, when it comes to the widgets tests that use this same data.) But currently we don't have any validation that that's the case. This issue is to add that validation.
This means:
We'll add a new field final String? url on ContentExample.
This field's value, when not null, should always be of the form https://chat.zulip.org/#narrow/stream/7-test-here/near/1774718.
I.e. it's a message on CZO in #test here.
The constructor should assert that it has this form, to maintain that invariant.
Leaving out the topic keeps down visual clutter when reading the source code.
The same information could be expressed with just the message ID, as an int. But having the whole URL can be handy for pasting into a browser to go look at how the message appears in web.
In the future we might loosen this to allow other Zulip servers, for cases where some realm config or server config makes a given output reproducible only outside CZO.
The url should be present wherever possible. When null, there should be a comment explaining why, just like for the existing markdown field.
For existing examples, we'll need to go back through and locate appropriate such messages — or just use the existing markdown field to send new ones.
Some examples will already have a valid URL in a comment, following this PR subthread.
We'll add some kind of check that confirms that for each example, if there is a non-null url, then it's valid: fetching the message from the actual chat.zulip.org server reports the same markdown and html that the example has.
This check should be possible to run from tools/check. Probably by default tools/check should run it but only on any examples that are new (or ideally those changed, too) relative to upstream. And it should run in CI… but possibly again only for new or changed examples.
The check should not run if you just say flutter test or flutter test test/model/ or the like.
(TBD how to handle credentials. We could set up in CI some credentials for a test account, but then there's still some setup for contributors to do if they're touching the content examples; maybe that's fine if we give the check a clear enough error message, with setup instructions. Alternatively we could make a web-public version of #test here and put the content-test messages there.)
The text was updated successfully, but these errors were encountered:
With the
ContentExample
class intest/model/content_test.dart
, we have a number of different examples of Zulip message content. These come in the form of HTML, because that's what the app consumes.It's important that for most of these tests, the HTML is something a Zulip server actually produced, because the job of these tests is to make sure we're successfully parsing whatever the server sends us, with all its quirks and sometimes messes. (Parsing and displaying, when it comes to the widgets tests that use this same data.) But currently we don't have any validation that that's the case. This issue is to add that validation.
This means:
final String? url
onContentExample
.https://chat.zulip.org/#narrow/stream/7-test-here/near/1774718
.#test here
.url
should be present wherever possible. When null, there should be a comment explaining why, just like for the existingmarkdown
field.markdown
field to send new ones.url
, then it's valid: fetching the message from the actual chat.zulip.org server reports the samemarkdown
andhtml
that the example has.tools/check
. Probably by defaulttools/check
should run it but only on any examples that are new (or ideally those changed, too) relative to upstream. And it should run in CI… but possibly again only for new or changed examples.flutter test
orflutter test test/model/
or the like.#test here
and put the content-test messages there.)The text was updated successfully, but these errors were encountered: