Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Sam: add tests to the public API #116

Merged
merged 3 commits into from
Jul 10, 2014
Merged

Sam: add tests to the public API #116

merged 3 commits into from
Jul 10, 2014

Conversation

MauricioCarneiro
Copy link
Contributor

Added the following tests:

  • unclipped start and stop
  • copy constructor tests
  • copy assignment test
  • tags of types char, int and double
  • char_tag when tag is not a char
  • string_tag when tag is not a string
  • is_missing support for tags

major strides towards addressing issue #4

Added the following tests:

* unclipped start and stop
* copy constructor tests
* copy assignment test
* tags of types char, int and double
* char_tag when tag is not a char
* string_tag when tag is not a string
* is_missing support for tags

major strides towards addressing issue #4
@MauricioCarneiro
Copy link
Contributor Author

@droazen can you take a peek ?

@coveralls
Copy link

Coverage Status

Coverage increased (+3.87%) when pulling df3b63f on mc_add_sam_tests into f999766 on master.

// check integer, char and float tags
const auto read1_za_tag = read1.double_tag("ZA");
BOOST_CHECK_EQUAL(read1_za_tag.name(), "ZA");
BOOST_CHECK(read1_za_tag.value() >= 2.3 - 0.001 && read1_za_tag.value() <= 2.3 + 0.001);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

float comparison with a tolerance should be a utility function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my laziness... BOOST_CHECK_CLOSE is for that exactly.

@droazen
Copy link
Contributor

droazen commented Jul 9, 2014

Review complete

@coveralls
Copy link

Coverage Status

Coverage increased (+4.12%) when pulling cc2bd17 on mc_add_sam_tests into f999766 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+4.01%) when pulling 423ad04 on mc_add_sam_tests into 7633174 on master.

@MauricioCarneiro
Copy link
Contributor Author

@droazen I'll rebase and merge at your command, sir.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.94%) when pulling b00746c on mc_add_sam_tests into 7633174 on master.

@droazen
Copy link
Contributor

droazen commented Jul 10, 2014

Looks good now -- merge away

droazen added a commit that referenced this pull request Jul 10, 2014
@droazen droazen merged commit cdafada into master Jul 10, 2014
@droazen droazen deleted the mc_add_sam_tests branch July 10, 2014 17:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants