[Makefile] Add pattern rules and simplify the test targets#1805
[Makefile] Add pattern rules and simplify the test targets#1805relokin wants to merge 2 commits into
Conversation
|
@maranget @HadrienRenaud this is WIP but I wanted to check with you if you would be ok with such a change in principle. I find that the Makefile has grown a bit too large and it's hard to understand what's going on. Hopefully by introducing some patterns we can make it shorter, easier to extend and easier to follow. |
HadrienRenaud
left a comment
HadrienRenaud
left a comment
There was a problem hiding this comment.
I'm generally very pro this.
(I haven't approved because I didn't check that there was no change in the actual commands)
| -conf ./herd/tests/instructions/AArch64.vmsa+ifetch/vmsa+ifetch.cfg \ | ||
| $(REGRESSION_TEST_MODE) | ||
| @ echo "herd7 AArch64 VMSA+ifetch instructions tests: OK" | ||
| test-all-asl:: test.herd-asl.cata.aarch64-VMSA |
There was a problem hiding this comment.
Is it on purpose that you've removed the -verbose argument?
There was a problem hiding this comment.
I didn't realise this was intentional. I will restore it unless you think it's not really necessary.
There was a problem hiding this comment.
I think it is necessary, thanks.
Thanks Hadrien! That's absolutely fine, just wanted to check if the direction of the patch is ok. I will do another pass. Perhaps one thing that I wanted to check with you, the ASL test targets are not as uniform as the rest, is the non-uniformity useful and intentional? |
I'm not sure what non-uniformity you are talking about. If you are talking about naming conventions, it is probably non-intensional. If you are talking about something else, I'm not sure. Could you expand or give an example? |
| @ echo "herd7 catalogue aarch64-faults tests (ASL): OK" | ||
|
|
||
| cata-test:: pick-test-mixed | ||
| pick-test-mixed: |
There was a problem hiding this comment.
@maranget just a quick question do you know what is the purpose of this test?
| -conf ./herd/tests/instructions/AArch64.vmsa+ifetch/vmsa+ifetch.cfg \ | ||
| $(REGRESSION_TEST_MODE) | ||
| @ echo "herd7 AArch64 VMSA+ifetch instructions tests: OK" | ||
| test-all-asl:: test.herd-asl.cata.aarch64-VMSA |
There was a problem hiding this comment.
I didn't realise this was intentional. I will restore it unless you think it's not really necessary.
relokin
force-pushed
the
makefile-cleanup
branch
from
223dff2 to
0ea09d5
Compare
relokin
force-pushed
the
makefile-cleanup
branch
3 times, most recently
from
9074895 to
b978005
Compare
diaolo01
left a comment
diaolo01
left a comment
There was a problem hiding this comment.
This cleanup is useful. Generally, looks good to me.
| @@ -0,0 +1 @@ | |||
| shelf.py No newline at end of file | |||
There was a problem hiding this comment.
I notice this shelf covers a much smaller set of tests than before. Is this intended?
There was a problem hiding this comment.
Does it? I think it's a soft link to shelf.py, so it should have the same set of tests as before.
There was a problem hiding this comment.
If I am reading this correctly, previously the makefile used to call shelf-test.py and now it calls shelf.py via ci-shelf.py. shelf has considerably less tests than shelf-test, which was deleted.
There was a problem hiding this comment.
Oh sorry I missed that. Should be fixed now.
relokin
force-pushed
the
makefile-cleanup
branch
from
b978005 to
08e3f99
Compare
Simplify the makefile but introducing pattern rules for herd tests. In addition, clean up the target names by introducing the following pattern for test targets: test.<herd/herd-asl/herd-mixed>.<cata/inst>.<suite> for example: test.herd.cata.aarch64-pick
Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
relokin
force-pushed
the
makefile-cleanup
branch
from
08e3f99 to
7ffc8b4
Compare
relokin
changed the title
relokin
changed the title
3 participants
Simplify the makefile but introducing pattern rules for the herd tests. In addition, clean up the target names by introducing the following pattern for test targets:
test..<cata/unit>.
for example:
test.herd.cata.aarch64-pick