Add tests for iterables #127

mar-celine · 2017-07-20T21:58:05Z

No description provided.

codecov-io · 2017-07-20T22:41:20Z

Codecov Report

Merging #127 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #127   +/-   ##
========================================
  Coverage    96.81%   96.81%           
========================================
  Files           72       72           
  Lines         2167     2167           
========================================
  Hits          2098     2098           
  Misses          69       69

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6db2f5d...0f74b0e. Read the comment docs.

nilsdeppe · 2017-07-20T23:16:20Z

tests/Unit/TestHelpers.hpp

+  CHECK(c.end() == c.cend());
+  CHECK(c.rend() - c.rbegin() == c.size());
+
+  auto it = c.begin();


what about the other begin and end cases? This only checks half. We also should have a test_iterators and a test_reverse_iterators because unordered associative containers do not have reverse iteration since there is no order defined.

kidder · 2017-07-21T19:29:04Z

tests/Unit/TestHelpers.hpp

+template <typename Container>
+void test_reverse_iterators(Container& c) {
+  CHECK(c.rend() - c.rbegin() == c.size());
+  auto it = c.begin();


check that crbegin and crend are equal to rbegin and rend

kidder · 2017-07-21T19:29:28Z

tests/Unit/TestHelpers.hpp

+
+  const auto& const_c = c;
+  CHECK(const_c.rend() - const_c.rbegin() == const_c.size());
+  auto cit = const_c.begin();


check that crbegin and crend are equal to rbegin and rend

kidder · 2017-07-21T19:30:20Z

tests/Unit/TestHelpers.hpp

+    cend--;
+  }
+}
+


add for loops checking the cbegin and cend

kidder

Also test TestHelpers on std::containers

wthrowe · 2017-07-23T17:41:03Z

tests/Unit/TestHelpers.hpp

+// Test for iterators
+template <typename Container>
+void test_iterators(Container& c) {
+  CHECK(c.end() - c.begin() == c.size());


A lot of stuff in here (such as the subtraction in this line) will only compile for random access iterators, but could be made to work for more using things like std::distance and std::next. std::set (for the bidirectional case) and std::unordered_set (for the forward case) can be used as tests of what works.

osheamonn · 2017-07-24T18:33:32Z

tests/Unit/TestHelpers.hpp

@@ -141,46 +141,71 @@ void test_move_semantics(T&& a, const T& comparison) {
 // Test for iterators
 template <typename Container>
 void test_iterators(Container& c) {
-  CHECK(c.end() - c.begin() == c.size());
+  // CHECK(c.end() - c.begin() == c.size());


Remove commented out code

CHECK(std::distance(c.begin(),c.end()) == c.size())
maybe with a cast

osheamonn · 2017-07-24T18:33:40Z

tests/Unit/TestHelpers.hpp

+  auto c_it = const_c.begin();
+  auto c_nx = std::next(c_it, std::distance(const_c.begin(), const_c.end()));
+  CHECK(c_nx == const_c.end());
+  // CHECK(const_c.end() - const_c.begin() == const_c.size());


Remove commented out code.

wthrowe · 2017-07-24T19:02:05Z

tests/Unit/TestHelpers.hpp

+  // CHECK(c.end() - c.begin() == c.size());
+  auto it = c.begin();
+  auto nx = std::next(it, std::distance(c.begin(), c.end()));
+  CHECK(nx == c.end());


Why was the check against c.size() removed? I don't think the new version tests much beyond that the std::distance call doesn't segfault or something.

remove 145-7

kidder · 2017-07-24T19:16:02Z

tests/Unit/TestHelpers.hpp

@@ -141,46 +141,71 @@ void test_move_semantics(T&& a, const T& comparison) {
 // Test for iterators
 template <typename Container>
 void test_iterators(Container& c) {
-  CHECK(c.end() - c.begin() == c.size());
+  // CHECK(c.end() - c.begin() == c.size());


CHECK(std::distance(c.begin(),c.end()) == c.size())
maybe with a cast

kidder · 2017-07-24T19:17:40Z

tests/Unit/TestHelpers.hpp

+  // CHECK(c.end() - c.begin() == c.size());
+  auto it = c.begin();
+  auto nx = std::next(it, std::distance(c.begin(), c.end()));
+  CHECK(nx == c.end());


remove 145-7

kidder · 2017-07-24T19:17:56Z

tests/Unit/TestHelpers.hpp

+  auto c_it = const_c.begin();
+  auto c_nx = std::next(c_it, std::distance(const_c.begin(), const_c.end()));
+  CHECK(c_nx == const_c.end());
+  // CHECK(const_c.end() - const_c.begin() == const_c.size());


kidder · 2017-07-24T19:18:10Z

tests/Unit/TestHelpers.hpp

+  CHECK(c.end() == c.cend());
+  CHECK(c.rend() - c.rbegin() == c.size());
+
+  auto it = c.begin();


kidder · 2017-07-24T19:21:12Z

tests/Unit/TestHelpers.hpp

  auto it = c.begin();
  auto rit = c.rbegin();
  auto end = c.end();
  auto rend = c.rend();
+  auto cit = c.cbegin();
+  auto cend = c.cend();
+  auto crit = c.rbegin();


c.crbegin();

kidder · 2017-07-24T19:21:27Z

tests/Unit/TestHelpers.hpp

+  auto cit = c.cbegin();
+  auto cend = c.cend();
+  auto crit = c.rbegin();
+  auto crend = c.rend();


kidder · 2017-07-24T19:23:00Z

tests/Unit/TestHelpers.hpp

-  auto crend = const_c.rend();
+  auto cr_it = const_c.rbegin();
+  auto cr_nx = std::next(cr_it, std::distance(const_c.begin(), const_c.end()));
+  CHECK(cr_nx == const_c.rend());


see size comment above

nilsdeppe requested changes Jul 20, 2017

View reviewed changes

mar-celine force-pushed the gab_test_iterator_addition branch from bd69d44 to af6ce8d Compare July 21, 2017 17:03

kidder reviewed Jul 21, 2017

View reviewed changes

tests/Unit/TestHelpers.hpp Outdated

cend--;

}

}

Copy link

Member

kidder Jul 21, 2017

Choose a reason for hiding this comment

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

add for loops checking the cbegin and cend

kidder requested changes Jul 21, 2017

View reviewed changes

wthrowe reviewed Jul 23, 2017

View reviewed changes

mar-celine force-pushed the gab_test_iterator_addition branch from 6b11c8e to 8ab6d1a Compare July 24, 2017 18:30

osheamonn reviewed Jul 24, 2017

View reviewed changes

wthrowe reviewed Jul 24, 2017

View reviewed changes

kidder requested changes Jul 24, 2017

View reviewed changes

mar-celine force-pushed the gab_test_iterator_addition branch from a1a45c8 to 0351bd1 Compare July 25, 2017 17:28

kidder previously approved these changes Jul 25, 2017

View reviewed changes

Add tests for iterables

0f74b0e

mar-celine dismissed kidder’s stale review via 0f74b0e July 25, 2017 19:06

mar-celine force-pushed the gab_test_iterator_addition branch from 0351bd1 to 0f74b0e Compare July 25, 2017 19:06

nilsdeppe approved these changes Jul 25, 2017

View reviewed changes

nilsdeppe merged commit e1356bd into sxs-collaboration:develop Jul 25, 2017

wthrowe mentioned this pull request Jul 27, 2017

Inconsistent warnings between machines #139

Closed

mar-celine deleted the gab_test_iterator_addition branch December 1, 2018 20:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add tests for iterables #127

Add tests for iterables #127

mar-celine commented Jul 20, 2017

codecov-io commented Jul 20, 2017 •

edited

Loading

nilsdeppe Jul 20, 2017

kidder Jul 24, 2017

kidder Jul 21, 2017

kidder Jul 21, 2017

kidder Jul 21, 2017

kidder left a comment

wthrowe Jul 23, 2017

osheamonn Jul 24, 2017

kidder Jul 24, 2017

osheamonn Jul 24, 2017

kidder Jul 24, 2017

wthrowe Jul 24, 2017

kidder Jul 24, 2017

kidder Jul 24, 2017

kidder Jul 24, 2017

kidder Jul 24, 2017

kidder Jul 24, 2017

kidder Jul 24, 2017

kidder Jul 24, 2017

kidder Jul 24, 2017

Add tests for iterables #127

Add tests for iterables #127

Conversation

mar-celine commented Jul 20, 2017

codecov-io commented Jul 20, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kidder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jul 20, 2017 •

edited

Loading