Improve testing of markdown.contains_backend_only_syntax()

[timabbott]

Currently, echo.contains_bugdown does a pretty good job of checking what bugdown is supported, but there are a few gaps.

However, we have some more obscure markdown features (e.g. !modal_link()) that are not checked for in the file. We have some test data in zerver/fixtures/markdown_test_cases.json as well as tests in zerver/tests/test_bugdown.py and frontend tests in frontend_tests/node_tests/echo.js. The test fixture data is preferred for new tests, since it is automated tested in both the test_bugdown and echo.js test suites.

We should do a few things:
(1) Document the above notes on how we test bugdown in docs/markdown.md
(2) Fill the gaps where markdown.contains_backend_only_syntax does not produce correct output, and add tests for this. The things I'm aware of include !modal_link and friends (see the issues below)
(3) Add a blueslip.warn call to report to the server whenever the local echo support ends up discovering a mismatch between marked and bugdown, so that we can catch future mismatches and fix or test them.

Some related issues / open PRs that may make sense to work on as part of this include:

• Add Bugdown tests for !avatar syntax #1654
• Add support for !avatar, !_subscribe_link, !modal_link in echo.js markdown #1653
• bugdown: Fix links immediately preceeded by an ! character. #390

@hackerkid is that enough info to get started on improving this area of Zulip?

[hackerkid]

@timabbott Think so. I will start working on this from next week. Have midsems coming up in a few days.

[timabbott]

Cool, good luck with your midterms!

[hackerkid]

Thanks 😄

[timabbott]

@hackerkid have you had a chance to look at this?

[hackerkid]

Yeah. I am working on #1653.

[timabbott]

Most of the side issues are now done, yay! The things that remain are:

• Add a blueslip.warn call to report to the server whenever the local echo support ends up discovering a mismatch between marked and bugdown, so that we can catch future mismatches and fix or test them.
• Documenting the testing notes about in docs/markdown.md.

[hackerkid]

@timabbott I was checking for some mismatches and found these two. I got them from the test cases in the function test_marked(frontend_tests/node_tests/echo.js).

Text: This is a @**Cordelia Lear** mention
Marked: <p>This is a <span class="user-mention" data-user-email="[email protected]">@Cordelia Lear</span> mention</p>
Bugdown: <p>This is a @<strong>Cordelia Lear</strong> mention</p>

Text: mmm...:burrito:s
Marked: <p>mmm...<img alt=":burrito:" class="emoji" src="/static/third/gemoji/images/emoji/burrito.png" title=":burrito:">s</p>
Bugdown: <p>mmm...:burrito:s</p>

Is comparing the output of Marked and Bugdown enough (something like echo.apply_markdown(message) === response_data.rendered) for deciding whether or not to report via blueslip.warn?

[timabbott]

Well I think places where echo.contains_bugdown determines the change does contain bugdown should not trigger the warning. I think the fact that those differ means those are actually bugs in our bugdown/marked setup.

So, you're right, I guess I summarized the issue in a relatively simple fashion; our goal is to find and cut down on these divergences, and the blueslip warning is a way to do that, but we might want to fix the known divergences first...

[hackerkid]

@timabbott I will start with upgrading the marked. After that will I will move to fixing divergences and adding blueslip call. Is this workflow sounds fine?

[timabbott]

Yep that sounds great!

[timabbott]

@hackerkid is there more work that we want to do on this issue, or should we close it?

[hackerkid]

@timabbott I am not entirely sure about that. We had a discussion regarding this here. https://chat.zulip.org/#narrow/stream/backend/topic/user-mention

[timabbott]

Yeah, OK, there's more to do. It might be worth using tools/test-js-with-node --coverage to produce a checklist of which frontend markdown code paths are untested currently; then we can try to clear those remaining checklist items :).

[hackerkid]

@timabbott Sure!

[timabbott]

I did some work on this in 0161671 and the few commits before it.

[timabbott]

I think basically everything we had in mind here is done. I'm pretty happy with the current testing system, and I think we decided the blueslip thing didn't totally make sense. If we have more trouble with this system, we can open a new issue.

I'm going to open issues for some follow-up projects, though.

[timabbott]

Opened #7099 for the main follow-up.