Cisco ospf cost

[dhalperi]

For #873.

This change is 

[progwriter]

---

Reviewed 13 of 13 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[arifogel]

Looks fine functionally. Some suggestions regarding tests.

---

Reviewed 13 of 13 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions, all commit checks successful.

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 117 at r1 (raw file):

            .getSummaries()
            .get(Prefix.parse("10.0.0.0/16"));
    assertThat(summary.getAdvertise(), is(false));

Please add newline, description of what is being asserted.
Also, I generally prefer that you use feature matchers for things like this, i.e. assertThat(summary, isAdvertise(false)) or assertThat(summary, not(isAdvertise()) (see InterfaceMatchers.isActive)

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 129 at r1 (raw file):

            .getSummaries()
            .get(Prefix.parse("10.0.0.0/16"));
    assertThat(summary.getAdvertise(), is(true));

Please add newline, description of what is being asserted.
Also, I generally prefer that you use feature matchers for things like this, i.e. assertThat(summay, isAdvertise(true)) or assertThat(summary, isAdvertise()) (see InterfaceMatchers.isActive)
and assertThat(summary, hasMetric(nullValue()))

---

Comments from Reviewable

[sfraint]

Reviewed 12 of 13 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[sfraint]

Review status: all files reviewed at latest revision, 3 unresolved discussions.

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 107 at r1 (raw file):

  @Test
  public void testSummaryMetricCost() throws IOException {

Just a nitpick here, but MetricCost seems redundant; maybe call it something like testOspfSummaryRouteMetric or just testOspfSummaryRoute instead?

---

Comments from Reviewable

[dhalperi]

Review status: 12 of 16 files reviewed at latest revision, 3 unresolved discussions.

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 107 at r1 (raw file):

Previously, sfraint (Spencer Fraint) wrote…

Just a nitpick here, but MetricCost seems redundant; maybe call it something like testOspfSummaryRouteMetric or just testOspfSummaryRoute instead?

Done.

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 117 at r1 (raw file):

Previously, arifogel (Ari Fogel) wrote…

Please add newline, description of what is being asserted.
Also, I generally prefer that you use feature matchers for things like this, i.e. assertThat(summary, isAdvertise(false)) or assertThat(summary, not(isAdvertise()) (see InterfaceMatchers.isActive)

Done.

---

projects/batfish/src/test/java/org/batfish/grammar/cisco/CiscoGrammarTest.java, line 129 at r1 (raw file):

Previously, arifogel (Ari Fogel) wrote…

Please add newline, description of what is being asserted.
Also, I generally prefer that you use feature matchers for things like this, i.e. assertThat(summay, isAdvertise(true)) or assertThat(summary, isAdvertise()) (see InterfaceMatchers.isActive)
and assertThat(summary, hasMetric(nullValue()))

Done.

---

Comments from Reviewable

[arifogel]

---

Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable