-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: GenerateCoverage Experiment #718
base: vara-dev
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## vara-dev #718 +/- ##
============================================
+ Coverage 68.55% 69.10% +0.54%
============================================
Files 338 344 +6
Lines 26434 28639 +2205
============================================
+ Hits 18122 19791 +1669
- Misses 8312 8848 +536
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm not 100% sure about the zip files for the tests. On the one side, it's good for testing, on the other, we don't know how to update/fix them when stuff breaks.
Maybe, we should at least put some doc/wording in a file that specifies how the information can be regenerated?
- I hope, this does not make the tests brittel
@boehmseb what's your take on that?
Overall, the implementation already looks quite good.
|
||
class TestCodeRegion(unittest.TestCase): | ||
|
||
def test_feature_config_report_map(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what this tests, the beginning looks a bit like testing the config handling part of the configuration, which should be tested in separation and in the config module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only tests if ConfigCoverageReportMapping
will use the right configs for diffing.
@@ -0,0 +1,708 @@ | |||
"""Code region tree and coverage report.""" | |||
|
|||
from __future__ import annotations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From which python version is this? As we have update to min 3.9 we maybe don't need this anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still necessary. Otherwise the following is not possible. See https://peps.python.org/pep-0563/ and https://peps.python.org/pep-0649/
class CodeRegion
def from(cls, x) -> CodeRegion:
...
return repr(self.x) | ||
|
||
|
||
class RunConfig(tp.FrozenSet[tp.Tuple[str, ConfigValue]]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need to build a config class here, is the one that parses the config format not working. If not, can we maybe adapt it?
We kind of brought this on ourselves by introducing .zip files as reports. |
As @boehmseb said. The zip files were obtained by copy pasting them from the results folder after running the |
That part I know and "buy", I'm more concerned to how we can update them if needed. We should add docu to the tests how to regenerate the files, project + experiment. By the way, as soon as we have a "docu format" or idea how to specify these, we should probably also document the other ones. |
I regenerated the files and noted down my steps. https://github.com/se-sic/VaRA-Tool-Suite/blob/fba20d72770514086ea65262a491378c5ea11963/tests/TEST_INPUTS/results/FeaturePerfCSCollection/HowToReproduce.md. Feedback welcome. |
…ow) or similar lines in plots,
…notated, once with profiling instructions.
…zip2, gzip, picosat, xz
…uced xz casestudy
No description provided.