<p></p>
<p><b>@lonvia</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/1249#discussion_r451794234">tests/test-output-flex-way-change.cpp</a>:</p>
<pre style='color:#555'>> +              "r30 v1 dV Tt=ag Mw10@mark,w11@,w12@mark,w13@,w14@mark"});
+
+    REQUIRE_NOTHROW(db.run_import(options, data()));
+
+    auto conn = db.db().connect();
+
+    CHECK(1 == conn.get_count("osm2pgsql_test_t1"));
+    CHECK(1 == conn.get_count("osm2pgsql_test_t2"));
+    CHECK(3 == conn.get_count("osm2pgsql_test_tboth"));
+    CHECK(1 == conn.get_count("osm2pgsql_test_tboth",
+                              "way_id = 10 AND rel_ids = '{30}'"));
+
+    options.append = true;
+
+    REQUIRE_NOTHROW(db.run_import(
+        options, "r30 v2 dV Tt=ag Mw10@,w11@,w12@mark,w13@,w14@mark"));
</pre>
<p>That boils to a relation change. Should already be covered by the relation change tests and can be removed here.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/1249#discussion_r451794297">tests/test-output-flex-way-change.cpp</a>:</p>
<pre style='color:#555'>> +              "r30 v1 dV Tt=ag Mw10@,w11@,w12@mark,w13@,w14@mark"});
+
+    REQUIRE_NOTHROW(db.run_import(options, data()));
+
+    auto conn = db.db().connect();
+
+    CHECK(1 == conn.get_count("osm2pgsql_test_t1"));
+    CHECK(1 == conn.get_count("osm2pgsql_test_t2"));
+    CHECK(3 == conn.get_count("osm2pgsql_test_tboth"));
+    CHECK(1 == conn.get_count("osm2pgsql_test_tboth",
+                              "way_id = 10 AND rel_ids IS NULL"));
+
+    options.append = true;
+
+    REQUIRE_NOTHROW(db.run_import(
+        options, "r30 v2 dV Tt=ag Mw10@mark,w11@,w12@mark,w13@,w14@mark"));
</pre>
<p>dto</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/1249#discussion_r451794730">tests/test-output-flex-way-change.cpp</a>:</p>
<pre style='color:#555'>> +    SECTION("change node list to make way invalid")
+    {
+        REQUIRE_NOTHROW(
+            db.run_import(options, "w10 v2 dV Tt1=yes,t2=yes,tboth=yes Nn10"));
+    }
+
+    SECTION("change node to make way invalid (n11 same location as n10)")
+    {
+        REQUIRE_NOTHROW(db.run_import(options, "n11 v2 dV x10.0 y10.0"));
+    }
+
+    CHECK(1 == conn.get_count("osm2pgsql_test_t1"));
+    CHECK(1 == conn.get_count("osm2pgsql_test_t2"));
+    CHECK(2 == conn.get_count("osm2pgsql_test_tboth"));
+    CHECK(0 == conn.get_count("osm2pgsql_test_tboth",
+                              "way_id = 10 AND rel_ids = '{30}'"));
</pre>
<p>Test for "way_id = 10" only, it's broader.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/1249#discussion_r451794982">tests/test-output-flex-way-change.cpp</a>:</p>
<pre style='color:#555'>> +{
+    options_t options = testing::opt_t().slim().flex(conf_file);
+
+    testing::data_t data{tdata};
+    data.add({"w10 v1 dV Tt1=yes,t2=yes,tboth=yes Nn10",
+              "r30 v1 dV Tt=ag Mw10@mark,w11@,w12@mark,w13@,w14@mark"});
+
+    REQUIRE_NOTHROW(db.run_import(options, data()));
+
+    auto conn = db.db().connect();
+
+    CHECK(1 == conn.get_count("osm2pgsql_test_t1"));
+    CHECK(1 == conn.get_count("osm2pgsql_test_t2"));
+    CHECK(2 == conn.get_count("osm2pgsql_test_tboth"));
+    CHECK(0 == conn.get_count("osm2pgsql_test_tboth",
+                              "way_id = 10 AND rel_ids = '{30}'"));
</pre>
<p>dto</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/1249#discussion_r451795977">tests/test-output-flex-way-change.cpp</a>:</p>
<pre style='color:#555'>> +}
+
+TEST_CASE("change invalid geom to valid geom")
+{
+    options_t options = testing::opt_t().slim().flex(conf_file);
+
+    testing::data_t data{tdata};
+    data.add({"w10 v1 dV Tt1=yes,t2=yes,tboth=yes Nn10",
+              "r30 v1 dV Tt=ag Mw10@mark,w11@,w12@mark,w13@,w14@mark"});
+
+    REQUIRE_NOTHROW(db.run_import(options, data()));
+
+    auto conn = db.db().connect();
+
+    CHECK(1 == conn.get_count("osm2pgsql_test_t1"));
+    CHECK(1 == conn.get_count("osm2pgsql_test_t2"));
</pre>
<p>General comment on T2 checks: I'm not convinced the test is sufficient. We should test that for all ways we have the right relations.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/osm2pgsql/pull/1249#discussion_r451799593">tests/test-output-flex-way-relation-add.cpp</a>:</p>
<pre style='color:#555'>> +    "n14 v1 dV x10.2 y10.0",
+    "n15 v1 dV x10.2 y10.1",
+    "n16 v1 dV x10.3 y10.0",
+    "n17 v1 dV x10.3 y10.1",
+    "n18 v1 dV x10.4 y10.0",
+    "n19 v1 dV x10.4 y10.1",
+    "w11 v1 dV Tt1=yes Nn12,n13",
+    "w12 v1 dV Tt2=yes Nn14,n15",
+    "w13 v1 dV Ttboth=yes Nn16,n17",
+    "w14 v1 dV Ttboth=yes Nn18,n19",
+    "w15 v1 dV Tt=ag Nn17,n19",
+    "r30 v1 dV Tt=ag Mw10@,w11@,w12@mark,w13@,w14@mark"};
+
+// adding relation (marked)
+
+TEST_CASE("test way: add relation with way in t1 (marked)")
</pre>
<p>Comment should reflect that we are testing relation change as well.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/osm2pgsql/pull/1249#pullrequestreview-445088875">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AA6353TAVV5W6I7TFIOFUL3R2TIQVANCNFSM4OUWA7LA">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AA6353U4RLOFQLUBTEEXHBLR2TIQVA5CNFSM4OUWA7LKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGODKDYI2Y.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/osm2pgsql/pull/1249#pullrequestreview-445088875",
"url": "https://github.com/openstreetmap/osm2pgsql/pull/1249#pullrequestreview-445088875",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>