RudreshVeerkhare / Catalyst

A VS code Extension to accelerate the process of solving problems on Codeforces.
GNU General Public License v3.0
77 stars 14 forks source link

Show standard error output in tests, and organise the problems better by keeping them under corresponding contest folders #9

Closed DrSwad closed 3 years ago

DrSwad commented 3 years ago

Feature 1: Show standard error output in tests

When the code is run and standard error output is not empty, the error output is shown in a separate box below the program output for each test case. This is very important for debugging purposes. Although the extension already showed the error outputs, it was shown concatenated with the problem output. Also, the extension used to mark the code as wrong if the standard error output wasn't empty even if the output matched with the expected output. Now, it only checks whether the outputs match or not.

Feature 2: Keep problems under corresponding contest folders

When creating a new contest, all the problems are stored in the root folder. If there are a lot of previous contest files and practice problems in the root folder already, finding the new contest's files would be really hard. The root directory also becomes very messy quickly. This commit solves that problem by scraping the contest name along with all the other information from the problem page and creating a folder with that contest name to store the problem in. This keeps the whole problem archive very organised and clean.

RudreshVeerkhare commented 3 years ago

Thank you @DrSwad For adding this changes... Contest folder feature is really needed one...glad you added it...

In feature 1 could you explain why should it be correct answer if STDERR is not empty but output matched. In my opinion STDERR not being empty represents error in code.

Also it'd be really helpful if you use "4 spaces" for formatting rather than "2 spaces" as it's showing unnecessary file changes and makes difficult to detect actual changes in code

DrSwad commented 3 years ago

Sorry, I was a bit occupied.

I'll change the indentation to 4 spaces in the next commit.

You're right about STDERR. My initial intention was actually to modify the code to suit my needs and didn't have the idea to create the pull request. But then thought that the changes might be useful for others using this extension. My coding flow usually goes this way:

  1. Fetch the problem.
  2. Solve and write an initial code, and run it against all the test cases.
  3. Failed a few tests.
  4. Analyse the failed tests and add some stderr print statements to some lines.
  5. Re-run only for the failed tests.
  6. If still failed, go back to step 3.
  7. Run code for all the tests.
  8. If failed, go back to step 3.
  9. Remove all the stderr print statements.
  10. Submit.

Now, the inconvenience here is at step 5. Since after adding some stderr, if I press Ctrl+Enter then all the test cases would run and show wrong answers and thus expand. Currently, stderr messages are trimmed past 300 characters so this might not seem like a big issue. But I do think that increasing it would be helpful since the stderr can very often pass even a thousand characters mark if you're debugging a vector/map/set (Suppose a problem requires character frequency count, then we need a vector with 26 elements. Printing it in the main loop could easily cross a thousand).

An alternative is to click on the Run button on a failed test after adding the stderr messages to run only that failed test. There are two small inconveniences here:

  1. Typing codes and then reaching for the button with the mouse to click on it, again modifying something and again reaching out for the button with the mouse, feels a bit wrong don't you agree? There probably should be a better UX that we're missing.
  2. Also, suppose after running the code I decided that I need another stderr message elsewhere. So I add it and in a hurry, press Ctrl+Enter. This time all the tests would expand, and there's no way to quickly collapse all the correct tests other than manually collapsing each one of them. This actually happened a lot to me, which is why I thought to just ignore stderr messages altogether.

I can suggest two solutions here, but I haven't properly thought about the possible repercussions yet. So your call :)

  1. If stdout matches with the answer, then collapse the input. But if the stderr is non-empty, then color the input as red. Color the input green only if the stdout matches the answer and also the stderr is empty. So far, I like this one. It solves all the inconveniences, easy to be aware of un-removed stderr messages before submitting by seeing the inputs colored red, and also very easy to implement.
  2. Have a separate command that ignores stderr and matches stdout only, maybe something like Ctrl+Shift+Enter, keeping it similar to the other command. This is safer, as it doesn't modify the already existing Ctrl+Enter command's behaviour and just adds a different command for the different purpose.
RudreshVeerkhare commented 3 years ago

Thanks @DrSwad for detailed explanation..

I get it.. It could get really tedious, debugging one test case and having affect it other right ones...
I think this problem can be solved if we use Ctrl + Shift + Enter for re-running only wrong cases...

This way debugging wrong cases would be more convenient.. and it won't affect previous right cases.

Let me know what you think about this one..

DrSwad commented 3 years ago

Sure, it's a good approach I think. But it can be implemented in a few different ways I guess. Let me iterate over each possibility:

  1. Run only the tests that failed the previous run, and consider non-empty stderr as errors: We use this new command only when we're debugging, so there will likely be stderr messages in all of the test outputs. So, this behaviour wouldn't help much in identifying the tests that actually failed in the current run and the ones that failed only due to non-empty stderr.
  2. Run only the tests that failed the previous run, and ignore stderr output: Solves the issue with the previous behaviour. Can't think of any additional catches atm, except that sometimes user's code modifications might make the code fail in previously passed tests. And in that case, forgetting to press Ctrl+Enter to check all the test cases one last time before submitting might cause WA. But the user has to press Ctrl+Enter anyway to make sure that all the stderr print statements are removed from the code.
  3. Run all the tests in every run, and ignore stderr output: This also works fine. It keeps the user updated as soon as his modification failed a test that was passed previously, so might be helpful. But it might also be inconvenient if the user just wants to focus and debug a specific test case and those other tests are just coming in the way.

Let me know which one you think might help the broader audience.

RudreshVeerkhare commented 3 years ago

I just checked Feature 2 implementation...
There's a major problem in it...
It's putting even single problem into folder of it's contest name... which in my opinion is unnecessary nesting..

Other Major problem is that it's not backward compatible... i.e. after changes previously loaded problem statements which are loaded before this feature, are not showing....

DrSwad commented 3 years ago

Single problems can be ignored from the feature easily if you prefer it that way. But I think many users (including me :) ) might actually wish to keep everything under corresponding folders and not mix files with folders. Do you think keeping an option for this behaviour in the settings would be a good idea?

Also, I'm not sure why previously fetched problems won't work correctly anymore. All this feature does is store the code files in separate folders, but all the data still come from the .catalyst file in the root folder like before. So, if previously fetched problems have stored their data in the .catalyst file, they should still be able to grab that data from there.

RudreshVeerkhare commented 3 years ago

I'm really sorry..I had made some configuration mistake that's why it failed In my case...😅

And you're right... It's a kind of a personal choice..and it'd be better to keep an option for this in settings, it would be more appropriate.

RudreshVeerkhare commented 3 years ago

Regarding to the STDERR problem...
I think most of users print debug statements in STDOUT only...

So mostly STDERR will be empty if execution is successful and will contain some message if there are errors in execution..
Let me know what you think about it...

DrSwad commented 3 years ago

I'm not sure if that's a fair assumption. I always thought printing debug statements in STDOUT is a bad idea. Since every time you want to check if your code is outputting correctly after some fixes, you'd have to comment out the debug statements. By any chance, if the code isn't correct yet, you'd have to uncomment all the debug statements again. It'd be a mess.

But that's a separate debate. As an extension, we should probably aim for a solution that at least works generally for everyone. Since it's unlikely anyone would be willing to change their coding practices to meet the requirements of an extension.

Also, I don't quite get your concern over ignoring STDERR in debug runs. If there are any errors in execution (RTE), then it's highly unlikely that the outputs would match.

RudreshVeerkhare commented 3 years ago

So I think final conclusion would be -

About this 2nd option I think using a command to run all testcases and ignore STDERR would be more appropriate as it'd reflect if new changes have affected previously correct test cases or not.

DrSwad commented 3 years ago

Great! BTW, how do you want the contest folder settings to work?

  1. Do you want contests to always have their own folders, and the settings to only apply to individual problems?
  2. Or do you want the settings to simply turn on/off the contest folder functionality, and when the settings is turned on both contests & individual problems would reside in contest folders?
  3. Or maybe both options in the settings? One for turning on/off the contest folder functionality. Another one for creating contest folders for individual problems too.
RudreshVeerkhare commented 3 years ago

I think it'll be better that contest to always have their own folders...
And in setting we'd keep an option to apply it to individual problems as well...

RudreshVeerkhare commented 3 years ago

It's been a long since this thread has become inactive... that's why I'm closing this pull request....

If you want to suggest any further changes you are welcome to do so.... Thank you....