CSUF-Computer-Science / project1

Project 1 - grocery checkout
MIT License
1 stars 20 forks source link

datafile_test.cpp has broken unit tests #1

Open Jay2645 opened 8 years ago

Jay2645 commented 8 years ago

In datafile_test.cpp, there are 2 occurrences where assert is called, but rather than pass in the threw variable, instead assert is called with the constant true, which will cause the test to always succeed.

{
    bool threw = false;
    try {
      load_products("not a filename");
    } catch (runtime_error e) {
      threw = true;
    }
    assert(true);
  }

  cout << "Invalid file format" << endl;
  {
    bool threw = false;
    try {
      load_products("datafile.hpp");
    } catch (runtime_error e) {
      threw = true;
    }
    assert(true);
  }

This section of the code isn't part of anything that will affect the project as a whole (as the code this is testing is given), but it's still something which should be looked at.

joshuaferrara commented 8 years ago

In addition, there is also this within datafile.hpp.

Within this file, getline is called from load_products. It seems on some systems, getline will not search for \r\n at the end of the line, rather it will only search for \n causing \r to be at the end of the string.

In my testing (on Ubuntu 14.04.4 LTS with clang-3.4) I was getting the extra 0x0D character (CR) at the end of the string retrieved by getline, causing a failure of local tests.

For example, this is the hexadecimal equivalent of "PRODUCTS DATABASE"

50524F4455435453204441544142415345

While this is what is returned by getline for the first line of the database

50524F44554354532044415441424153450D

This could be solved by searching for and replacing all common line endings, if any are in the returned line. Or running a tool to convert the line endings. Nevertheless, posted in case anyone was having the same issues.

Jay2645 commented 8 years ago

I was having the issue with getline returning an extra junk character at the end of each line when compiling on my Windows machine as well. I wound up working around it by running the unit tests on the Ubuntu MATE VM that was referenced in the assignment instructions, where it worked without issues.

Since I'm assuming the tests would be running on that Ubuntu MATE distribution, it shouldn't matter for the purposes of this assignment, but anyone else having a similar issues that doesn't want to play around with modifying datafile.hpp or changing all the line endings in the products.dat file should just try their unit tests within that VM to verify they work.