Conversation
|
Hi @asherbhs! Thank you for the PR. 💯 The code coverage document lives on the wiki (for obvious reasons). You can find it here: https://github.com/Dyalog/ullu/blob/main/docs/code-coverage.md |
|
@asherbhs, I don't think I will have time this week to look at the PR in detail and I am on holiday next week. Are you in a rush to get this reviewed? |
|
no worries, I'm not in a hurry to get it merged. I might even get a chance to add even more tests! |
|
Hi @asherbhs I had a quick look and it looks great over all. I will take a more in-depth look later as well. One thing I noticed is that the entire test runs with ⎕IO set to 0, which means we don't test the cases where ⎕IO is 1. It is of course OK that your model has a fixed IO. Perhaps something similar happens for some of the other system variables. Do you think you could wrap the entire test in a loop, or something similar, so that your test cases run with both values of ⎕IO (and other system variables where it makes sense)? I know it sometimes seems like it doesn't matter, because your ⎕IO 0 code might hit all the interesting cases already, but all the other ullu tests generally try to cover all combinations of ⎕IO,⎕CT,⎕FR and so on, just to get more coverage. Who knows, perhaps there is a bug in stencil that only shows itself once ⎕IO is 1 and ⎕CT is 0 for example :) |
And indeed we have! See Mantis 19799 for example |
|
Thanks @pmikkelsen and @bear8642, all ⎕FR settings are covered, but we could cover more ⎕CT values in the general case (⎕CT has to be 0 to hit some of the special cases), and we should definitely test with all values of ⎕IO and ⎕ML. I'll get those added before this is merged |
Description
Add tests for
⌺, both general and special cases. You will need a recent build of the trunk for these tests to run without failures.PR Checklist (Remove options that are not relevant)