dlmarion commented on code in PR #53:
URL: https://github.com/apache/accumulo-access/pull/53#discussion_r1489470295
##########
src/it/antlr4-example/src/test/java/org/apache/accumulo/access/grammar/SpecificationTest.java:
##########
@@ -30,23 +30,21 @@
import org.antlr.v4.runtime.LexerNoViableAltException;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.Recognizer;
+import org.apache.accumulo.access.AccessExpression;
import org.apache.accumulo.access.grammar.antlr.Antlr4Tests;
import org.apache.accumulo.access.grammars.AbnfLexer;
import org.apache.accumulo.access.grammars.AbnfParser;
import org.junit.jupiter.api.Test;
// This test uses the ANTLR ABNF grammar to parse the
-// accumulo-access ANBF specification to validate that
+// AccessExpression ANBF specification to validate that
// it is proper ANBF.
public class SpecificationTest {
@Test
public void testAbnfSpecificationParses() throws Exception {
- // The test resource specification.abnf is a copy of the ABNF
- // from SPECIFICATION.md
-
- InputStream is =
Antlr4Tests.class.getResourceAsStream("/specification.abnf");
+ var is =
AccessExpression.class.getResourceAsStream("specification/AccessExpression.abnf");
Review Comment:
I think we could remove `specification/AccessExpression.abnf` and pull the
abnf specification from `SPECIFICATION.md` with the changes that you made in
`AccessExpressionTest.testSpecificationDocumentation`.
##########
src/test/java/org/apache/accumulo/access/AccessExpressionTest.java:
##########
@@ -189,4 +198,30 @@ public void testEqualsHashcode() {
assertEquals(ae1.hashCode(), ae3.hashCode());
assertNotEquals(ae1.hashCode(), ae2.hashCode());
}
+
+ @Test
+ public void testSpecificationDocumentation() throws IOException,
URISyntaxException {
+ // verify AccessExpression.abnf matches what is documented in
SPECIFICATION.md
+
+ // read the abnf spec, ignoring the header
+ List<String> specLinesFromAbnfFile;
+ try (
+ var abnfFileStream =
+
AccessExpression.class.getResourceAsStream("specification/AccessExpression.abnf");
+ var inputStreamReader = new InputStreamReader(abnfFileStream, UTF_8);
+ var bufferedReader = new BufferedReader(inputStreamReader)) {
+
+ Predicate<String> abnfComment = line -> line.startsWith(";");
+ Predicate<String> beforeFirstLine = abnfComment.or(String::isBlank);
+ specLinesFromAbnfFile =
bufferedReader.lines().dropWhile(beforeFirstLine).collect(toList());
+ }
+
+ // grab from the markdown, but make sure to skip the markdown triple ticks
Review Comment:
IIRC, the `AccessExpression.abnf` file exists solely so that we can test
that it's valid ABNF. We were unable to figure out how to embed
`AccessExpression.abnf` into `SPECIFICATION.md`. Given that you were able to
just pull the ABNF out of `SPECIFICATION.md`, then I think we could probably
get rid of `AccessExpression.abnf` and `SpecificationTest` (or just move this
test method to `SpecificationTest`.
This is a great change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
