Unexpected test error

Platform Feature Requests
1

I have been trying the new history extension for shell, but some of my submissions fail unexpectedly on the redirection stages. Resubmitting the solution via a blank commit makes them pass.

Here are the logs of some of the failures I have encountered:

image
image978×693 69.9 KB

image
image978×352 80.3 KB

The common thing you can see in these runs: the last command in all of them is using the redirection functionality with an unmatched quote '! In this a bug in the tester (as sometimes I do face this unmatched quote error) or is there probably something wrong in my implementation?

1 Like
3

My bad. Seems like my shell is register typing capital A as an up arrow press. That causes the shell to shift back to the previous command, and causes the incorrect command formation as seen in the above screenshots. :frowning:

Update: Fixed it successfully. The issue was I had believed co-pilot that the key code for Arrow Up for 65, and that for key down is 66. This works only half of the time (as the actual arrow up and arrow down key presses contain multiple bytes: ESCAPE [ A and ESCAPE [ B respectively), but fails any other tests if:

  • The command executed has a capital A or B in them
  • The command is not the first command being executed

This seems like an easy way to shoot yourself in the foot, and I wouldn’t be surprised if many other people, too, have this bug in this implementation. The current tests for the history stage are NOT designed to catch this bug, but it is sometimes caught in the redirection stages (if the name being used in some command is Alice). This issue of being caught in some other tests, combined with the fact that it only happens sometimes, makes it super notorious.

It would be nice if in the stage 4 (#RH7) and 5 (VQ0) itself of this challenge we add test cases to explicitly check that the shell is only going back and forward in history on the complete arrow press, and not any sub-sequence of it (like A, B, ESCAPE A, ESCAPE B etc) as it would help to catch these common buggy implementations earlier.

Thanks for a great extension!
Tagging for visibility and opinions: @andy1li @Arpan-206

1 Like
4

Hey @EshaanAgg, I’ll pass your suggestion to the team to explore adding tests for this case. Glad to hear you were able to fix it!

1 Like