talent-plan icon indicating copy to clipboard operation
talent-plan copied to clipboard

Project 2: Tests cli_get_non_existent_key and cli_rm_stored disagree with specification

Open loewenheim opened this issue 4 years ago • 3 comments

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. What did you do?

Implemented kvs according to the spec and ran the tests.

2. What did you expect to see?

Expected the tests to pass (insofar as my implementation was complete)

3. What did you see instead?

These tests fail because they expect the program to exit with a success in the case of a nonexistent key. This is explicitly counter to the spec, which says that when a key is not found, a non-zero exit code should be returned.

loewenheim avatar Apr 24 '20 16:04 loewenheim

I am not sure if it is the spec incorrect/ambiguous:

Under "Project Spec", it mention to return non-zero exit code on failure.

• kvs get <KEY>

  Get the string value of a given string key.
  Print an error and return a non-zero exit code on failure.

Later on in "Part 2 How the log behaves":

• "get"
  • The user invokes kvs get mykey
  • kvs reads the entire log, one command at a time, recording the
    affected key and file offset of the command to an in-memory key -> log
    pointer map
  • It then checks the map for the log pointer
  • If it fails, it prints "Key not found", and exits with exit code 0
  • If it succeeds
    • It deserializes the command to get the last recorded value of the key
    • It prints the value to stdout and exits with exit code 0

The 4th point state that if it fails to find the key in the map, print "Key not found" and exit with code 0.


So, this is how I interpret it:

If a key is not found, print "Key not found" and exit with code 0. This also implies that when a key is not found, it is not a failure.

If there is a failure on the get command, return non-zero exit code, where the failures here refer to failure to read files, incorrect inputs provided and etc.

I might be wrong here though. Just my 2 cents here, hope it helps 👍

kw7oe avatar Sep 07 '20 14:09 kw7oe

My interpretation is that these parts of the specification contradict each other. If your interpretation is correct, then I think the distinction between the two senses of fail/failure should be made more explicit.

loewenheim avatar Sep 07 '20 14:09 loewenheim

Yeap agree on that 👍 It should be made more explicit if that's the case.

kw7oe avatar Sep 07 '20 14:09 kw7oe