Open gregveres opened 4 years ago
@Raathigesh [update: see the last message in this thread. I found a very low change way to address this]
I started looking at a solution to this, but my knowledge of react and apollo is limited. I think the real problem here is that test-file/index.tsx is a functional component rather than a class based component. My understanding of the functional component is that they have no context and have to create the output completely from the input. But in this case, the only input they are given is now the "no result" result from the "changeToResult" subscription.
I was hoping that each time there was a result, that I could save that result and use this saved version when the changeToResult passed me an "empty result". But there is no where to save the result. I guess I could create a variable outside the function and save it there. This will work since there is only a single instance of the TestFile. If it was a class component, I would make it a class instance.
Since you understand the architecture better, I wonder if you have an idea of a better way to solve the issue.
I note that the event of starting a new test run does not have its own subscription. Instead, it seems to issue a changeToResult event with an empty result. That is what I was triggering the isUpdated on. But a better solution might be to change the events / subscriptions so that instead of the changeToResult, there was actually a testRunStarted event and a testRunFinished event. In that case, the testRunStarted wouldn't send a test result and the result would be sent in the testRunFinished event.
This would let us set isUpdating true on the testRunStarted and false on the testRunFinished.
Actually as I talk this through, I realize that I don't really understand how the functional component works with respect to these events.
Anyway, some guidance would be appreciated. With this change, I think Majestic becomes a fantastic workflow for working on a single file.
OK, creating a cached result that is at the file scope for test-file/index.tsx works, but it feels hacky.
I was playing around a bit more and I see that when I switch between different tests, the client asks the server for the results. So the server is caching the results per file somewhere. This leads me to think that changing the server events is the better change. And that starting a new test run shouldn't invalidate the test cache on the server until the test run result comes back.
I will see if I can learn how all that works on the server side.
Already I think I have found a solution. In workspace/resolver.ts I see there are two types of event Ids that trigger this subscription: TEST_RESULT and TEST_START. So instead of creating a brand new, empty TestFileResult for the TEST_START, I think the solution is to look up this files latest test result and if we find one, then send it along in the new empty TestFileResult. This provides the UI with an empty summary but the previous test results. This allows the isUpdating from the previous PR to still catch this test start state and it provides the previous test results so they are still shown.
I think I am going to see if I can "grey out" the test results during the test run so that there is yet another visual cue that these are old test results that will change soon. I should have a PR for you to look over soon. (although I have messed up my branches in my fork). I have been trying to keep a branch that corresponds to your fork and one that has my collapsableUI and any other changes as my fork's master.
@Raathigesh There are a couple bugs with this commit. I am noticing some cases that aren't handled. I am going to articulate them here and when I get a chance, I will see about fixing them. As I encounter others related to this change, I will add them to the list and get them fixed - hopefully by the end of the weekend:
Is this a bug report or a feature request?
This is a feature request that is built on top of PR #193. I have been running Majestic for a few days with the new style summary header. I like it, but I find that my workflow is the following:
But as soon as I hit save in step #2, the results from the previous run vanish. Now that it is really easy to see that a test is running, it would be nice if the results from the last run were still on the screen. If I was running in a terminal console, I would be able to just scroll back in the buffer to see the previous results so I could start working on the next failing test while the tests continued to run.
But right now with the tests vanishing, I have to wait for the test run to finish before I can see what the next failing test is. It would be nice if the test result didn't vanish while the test is running.
Version Info
Reproduction Repo
At this point you will notice that the test result (with the failures) vanishes as the summary background is now animated to indicate that the test is running. It would be nice if the test results were still shown so I could look at the next failure while the test is running.