Skip to content

Conversation

@moritz-gross
Copy link
Contributor

as you mentioned earlier, it'd be interesting to have some form of coverage report on the rules.
I added logic for this to src/rule_coverage.rs, which gets used in speech.rs if the Cargo feature rule-coverage is on.
The coverage is saved to the format provided by LCOV, which is what I already explored previously for the vanilla test coverage.

With cargo test --features rule-coverage, the LCOV files can be created, and genhtml --flat target/rule-coverage/lcov.info -o target/rule-coverage/html-flat then creates a report in HTML format that can be viewed.

Right now, it looks pretty empty (maybe something is missing?):
image

I spent only limited effort on validating the mutex-logic and export to LCOV (which I've never used before), but I think the general idea of using a Cargo feature and then piping the results into existing coverage tooling is a good approach.

We can discuss and refine this later of course, I just couldn't help myself but try to hack this together :)

…t coverage

- Introduced a `rule-coverage` Cargo feature to enable detailed tracking of rule hits in YAML files for speech, braille, and intent rules.
- Added `rule_coverage.rs` for managing registration, hit recording, and exporting LCOV-compatible coverage data.
- Integrated rule-coverage hooks into `speech.rs` for rule hit recording and ID registration.
- Updated `Cargo.toml` to include the new feature.
@moritz-gross
Copy link
Contributor Author

just saw that CI failed. parts of it are unrelated to my changes I think: https://github.com/TalkingCatSW/MathCAT/actions/runs/21458974397/job/61806814200#step:3:175


error: called `unwrap` on `operator_versions.postfix` after checking its variant with `is_some`
    --> src/canonicalize.rs:3561:12

@NSoiffer
Copy link
Collaborator

Using a feature is a good way to go.

I'll need to look into the error you got.

For the rule coverage, I think you are only recording that a rule is used, not which parts of it gets used. Recording that info is definitely better than nothing, but it isn't saying (for example) whether the "then" branch or the "else" is used. I didn't look in close detail, but maybe that is why the line coverage is so low?

@moritz-gross
Copy link
Contributor Author

yep, this not treating rules on line-level.
I'll look at that at some later point.

@NSoiffer
Copy link
Collaborator

MathCAT's rule interpreter is basically just a recursive descent interpreter. Most things will have a build and a replace function. The replace functions are where you could add some instrumentation. For example, The TestArray class's (- test:) replace function is

    fn replace<'c, 's:'c, 'm:'c, T:TreeOrString<'c, 'm, T>>(&self, rules_with_context: &mut SpeechRulesWithContext<'c, 's,'m>, mathml: Element<'c>) -> Result<T> {
        for test in &self.tests {
            if test.is_true(&rules_with_context.context_stack.base, mathml)? {
                assert!(test.then_part.is_some());
                return test.then_part.as_ref().unwrap().replace(rules_with_context, mathml);
            } else if let Some(else_part) = test.else_part.as_ref() {
                return else_part.replace(rules_with_context, mathml);
            }
        }
        return T::from_string("".to_string(), rules_with_context.doc);
    }

If a count were kept, you would be able to record the 'i'th then/else was taken (if then ... else_if ...then ... else_if ...). I don't know how you relate that back to the line number though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants