Refactor tests on modularized code structure #316
Conversation
…o package private
…o package private
… to package private
AntoniusW
changed the title
|
@madmike200590 Since you closed the other PR due to a "broken history", I have to ask whether the history we have here is the intended one? It seems to contain all commits from the modularization (again). Is rebasing a good (and easily executable) idea for this branch? |
Yes, the history here is the intended one. I closed the other PR because I had some issues getting git to start a proper merge locally - in hindsight, that was mostly my own mistake, but I wanted to make sure that the "shifted" (since modules was merged while #308 was open) base of the PR was not to blame. As for rebasing, yes, that might be elegant - I tried and reverted it since it's unfortunately not easily executable. The reason for my merging troubles was that we have one squashed commit for PR #274 in master, while we have the individual commits for the same changes plus commits further modifying the same files here. Since the history git would need to know that the changes from here have to be applied on top of what's in master is not available (because it got squashed), I had to do a huge manual merge. As for merging this PR, that means I'd like to do a simple merge and keep the history as it is here - otherwise we'll have the same problem with every branch that got branched from modules before merging PR #274. I'll investigate if we can still get rid of "WIP" commits by selectively squashing smaller sets of commits. |
Codecov Report
@@ Coverage Diff @@
## master #316 +/- ##
============================================
+ Coverage 69.51% 70.38% +0.87%
- Complexity 2092 2135 +43
============================================
Files 182 182
Lines 8006 8016 +10
Branches 1416 1417 +1
============================================
+ Hits 5565 5642 +77
+ Misses 2086 1996 -90
- Partials 355 378 +23
Continue to review full report at Codecov.
|
alpha-solver/src/test/java/at/ac/tuwien/kr/alpha/api/impl/RuleParser.java
Outdated
Show resolved
Hide resolved
alpha-solver/src/test/java/at/ac/tuwien/kr/alpha/api/impl/RuleParser.java
Outdated
Show resolved
Hide resolved
…Parser.java Co-authored-by: Antonius Weinzierl <[email protected]>
…Parser.java Co-authored-by: Antonius Weinzierl <[email protected]>
| @@ -75,7 +75,7 @@ | |||
| /** | |||
| * The new default solver employed in Alpha. | |||
| * | |||
| * Copyright (c) 2016-2021, the Alpha Team. | |||
| * Copyright (c) 2016-2020, the Alpha Team. | |||
There was a problem hiding this comment.
| * Copyright (c) 2016-2020, the Alpha Team. | |
| * Copyright (c) 2016-2021, the Alpha Team. |
| assertEquals(1, (int)newAssignmentsIterator.next()); | ||
| assertEquals(2, (int)newAssignmentsIterator.next()); |
There was a problem hiding this comment.
Please revert all changes to this file, it re-introduces compiler warnings that were fixed in master. It seems no other changes (besides the re-shuffling of import statements) were made, and the file could be reverted completely.
| @@ -125,7 +126,7 @@ public void parseInterval() { | |||
| ASPCore2Program parsedProgram = parser.parse("fact(2..5). p(X) :- q(a, 3 .. X)."); | |||
| IntervalTerm factInterval = (IntervalTerm) parsedProgram.getFacts().get(0).getTerms().get(0); | |||
| assertTrue(factInterval.equals(IntervalTerm.getInstance(Terms.newConstant(2), Terms.newConstant(5)))); | |||
| IntervalTerm bodyInterval = (IntervalTerm) parsedProgram.getRules().get(0).getBody().stream().findFirst().get().getTerms().get(1); | |||
| IntervalTerm bodyInterval = (IntervalTerm) ((Literal) parsedProgram.getRules().get(0).getBody().stream().findFirst().get()).getTerms().get(1); | |||
There was a problem hiding this comment.
Please revert this file, it re-introduces a compiler warning that was fixed already and no other relevant changes seem to have occurred in this file.
(clone of PR #308)
This PR is a follow-up to PR #274. Following the changes to the overall code structure and API introduced through modularization, I took the opportunity to get started on some refactorings of Alphas unit tests.
Specifically, this PR aims to
Reduce dependencies and narrow down tested code for unit tests
In order to make our unit tests more precise and avoid localized errors failing lots of tests, I removed unnecessary dependencies from most tests. Especially, we don't want to have tests of core components like gronder and solver depend on the complete program preprocessing workflow including parser and rewritings.
Move functional tests to alpha-solver module
Most tests that test rewriting and preprocessing features, as well as all tests that just pass programs into an
Alphainstance and verify returned answer sets should be considered functional tests rather than unit tests since they test complete features through the complete workflow. These were moved into thealpha-solvermodule since the default implementation ofAlpharesides there.Mark functional and integration tests for further attention
In order to improve build performance and better distinguish between unit-, integration-, functional- and performance-tests, these should be separated into separate tasks in the build (of which some should probably only run in CI) process.
This has not yet been realized yet, but tests that should probably be treated as integration tests or functional tests were marked with appropriate
TODOs in preparation for that refactoring step. (Issue TBD)