Skip to content
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

Add .cairo-coverage-ignore tests #110

Merged
merged 1 commit into from
Dec 6, 2024
Merged

Add .cairo-coverage-ignore tests #110

merged 1 commit into from
Dec 6, 2024

Conversation

ksew1
Copy link
Member

@ksew1 ksew1 commented Dec 4, 2024

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

Copy link
Member

@THenry14 THenry14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be cool to also test the edge cases related to the /~https://github.com/software-mansion/cairo-coverage/pull/109/files (not necessarily e2e tests, could be just unit)

As an example:

  • correct errors are thrown for CairoCoverageIgnoreMatcher::new (file does not exist?)
  • does find_ignore_file really work for things up to root?

etc

@ksew1 ksew1 force-pushed the spr/main/9bc72f44 branch from f144fcc to 0618714 Compare December 5, 2024 15:45
@ksew1 ksew1 changed the title Add tests Add .cairo-coverage-ignore tests Dec 5, 2024
ksew1 added a commit that referenced this pull request Dec 5, 2024
Base automatically changed from spr/main/9bc72f44 to main December 5, 2024 15:56
@ksew1 ksew1 force-pushed the spr/main/f6098cdc branch 2 times, most recently from 5845fe8 to 7e6f968 Compare December 6, 2024 12:10
@ksew1
Copy link
Member Author

ksew1 commented Dec 6, 2024

it would be cool to also test the edge cases related to the /~https://github.com/software-mansion/cairo-coverage/pull/109/files (not necessarily e2e tests, could be just unit)

As an example:

* correct errors are thrown for CairoCoverageIgnoreMatcher::new (file does not exist?)

* does find_ignore_file really work for things up to root?

etc

Really good idea 😄. Added this

@ksew1 ksew1 requested a review from THenry14 December 6, 2024 12:17
Copy link
Member

@THenry14 THenry14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🫵🏻 🪨

commit-id:f6098cdc
@ksew1 ksew1 force-pushed the spr/main/f6098cdc branch from 7e6f968 to 1b75b4b Compare December 6, 2024 13:17
@ksew1 ksew1 merged commit 01ef4b3 into main Dec 6, 2024
5 checks passed
@ksew1 ksew1 deleted the spr/main/f6098cdc branch December 6, 2024 13:39
ksew1 added a commit that referenced this pull request Dec 9, 2024
<!-- Reference any GitHub issues resolved by this PR -->

Related
#110 (comment)

## Introduced changes

<!-- A brief description of the changes -->

- bump snforge in Scarb.toml and .tools-versions

## Checklist

<!-- Make sure all of these are complete -->

- [ ] Linked relevant issue
- [ ] Updated relevant documentation
- [ ] Added relevant tests
- [x] Performed self-review of the code
- [ ] Added changes to `CHANGELOG.md`
ksew1 added a commit that referenced this pull request Dec 9, 2024
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.

Add .cairocoverageignore (subject to change)
2 participants