This repository has been archived by the owner on Sep 30, 2024. It is now read-only.
Add tests to ensure we do not again have webapp regression: "Syntax-highlighting this file took too long." is not falling back to plaintext rendering mode #4267
Comments
slimsag
added
bug
|
What kind of test would prevent this sort of regression in the future? Unit, dogfooding, load test, e2e? |
slimsag
added a commit
that referenced
this issue
Jun 5, 2019
This is useful for confirming our behavior of highlighting timeouts, and can be used in the future to automate testing of this behavior as part of an e2e test. Primarily adding to reproduce https://github.com/sourcegraph/sourcegraph/issues/4267 Note: This matches other cases where e.g. github.com/sourcegraphtest/AlwaysCloningTest is a special repository for testing gitserver cloning states end-to-end. Helps #4267
slimsag
added a commit
that referenced
this issue
Jun 5, 2019
A regression (which must've occurred awfully long ago, like around 8mo ago) means that today when syntax highlighting takes too long (which can happen for multiple reasons: due to very large files, bugs in the language grammar, or bugs in syntect itself) we are incorrectly just displaying the message and action: > Syntax-highlighting this file took too long. [Try again] And despite the fact that the server returned an unhighlighted version of the file, we're just discarding it instead of presenting it to the user as this system was originally designed / intended to work. What this means in practice is that today Sourcegraph is very unusable on large files because as soon as you view them you get the above error and have to hit `Try again` and wait a very long time. Instead, we now once again properly show the above error and action (for users who are willing to wait longer for syntax highlighting on these files) AND the actual plaintext version of the file. How it looks:  Fixes #4267 See also #4364 which adds something that will allow us to e2e test this in the future.
|
https://github.com/sourcegraph/sourcegraph/pull/4364 for how we can test this in the future (e2e test). |
|
Keeping open for proper e2e + unit testing of this. |
slimsag
removed
customer
slimsag
changed the title
slimsag
added a commit
that referenced
this issue
Jul 2, 2019
* blob: correct handling of syntax highlighting timeouts A regression (which must've occurred awfully long ago, like around 8mo ago) means that today when syntax highlighting takes too long (which can happen for multiple reasons: due to very large files, bugs in the language grammar, or bugs in syntect itself) we are incorrectly just displaying the message and action: > Syntax-highlighting this file took too long. [Try again] And despite the fact that the server returned an unhighlighted version of the file, we're just discarding it instead of presenting it to the user as this system was originally designed / intended to work. What this means in practice is that today Sourcegraph is very unusable on large files because as soon as you view them you get the above error and have to hit `Try again` and wait a very long time. Instead, we now once again properly show the above error and action (for users who are willing to wait longer for syntax highlighting on these files) AND the actual plaintext version of the file. How it looks:  Fixes #4267 See also #4364 which adds something that will allow us to e2e test this in the future. * Update web/src/repo/blob/BlobPage.tsx Co-Authored-By: Felix Becker <[email protected]>
slimsag
added a commit
that referenced
this issue
Jul 3, 2019
This is useful for confirming our behavior of highlighting timeouts, and can be used in the future to automate testing of this behavior as part of an e2e test. Primarily adding to reproduce https://github.com/sourcegraph/sourcegraph/issues/4267 Note: This matches other cases where e.g. github.com/sourcegraphtest/AlwaysCloningTest is a special repository for testing gitserver cloning states end-to-end. Helps #4267
|
@slimsag is this resolved? If not, do you think it will be ready for branch cut on Monday? |
slimsag
added
debt
|
Backlogging. This is for tests. |
slimsag
added a commit
that referenced
this issue
Jul 13, 2019
This is useful for confirming our behavior of highlighting timeouts, and can be used in the future to automate testing of this behavior as part of an e2e test. Primarily adding to reproduce https://github.com/sourcegraph/sourcegraph/issues/4267 Note: This matches other cases where e.g. github.com/sourcegraphtest/AlwaysCloningTest is a special repository for testing gitserver cloning states end-to-end. Helps #4267
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
When the syntax highlighter bails out because highlighting is taking too long (e.g. on large files or files it can't handle well for whatever reason), we always return a plaintext version of the document to be rendered.
However, it appears there has been a regression where the webapp / TS code is incorrectly ignoring the plaintext form it got instead of rendering it as it should be:
https://sourcegraph.com/github.com/sourcegraph/sourcegraph@cdedd0e83a68a314f67d53e261209e272d787735/-/blob/web/src/repo/blob/BlobPage.tsx#L275
Reported by https://app.hubspot.com/contacts/2762526/company/407948923
The text was updated successfully, but these errors were encountered: