Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

content test: Record URLs of sample messages, and validate them #617

Open
gnprice opened this issue Apr 4, 2024 · 0 comments
Open

content test: Record URLs of sample messages, and validate them #617

gnprice opened this issue Apr 4, 2024 · 0 comments
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents a-tests a-tools Our own development tooling, scripts, and infrastructure

Comments

@gnprice
Copy link
Member

gnprice commented Apr 4, 2024

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.)
@gnprice gnprice added a-content Parsing and rendering Zulip HTML content, notably message contents a-tools Our own development tooling, scripts, and infrastructure a-tests labels Apr 4, 2024
@gnprice gnprice added this to the Beta 3 milestone Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-content Parsing and rendering Zulip HTML content, notably message contents a-tests a-tools Our own development tooling, scripts, and infrastructure
Projects
Status: No status
Development

No branches or pull requests

1 participant