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

testing: (stencil) remove stencil printing test #3957

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

superlopuh
Copy link
Member

Is there a need for this test any more?

I'm not sure what it's testing, it seems to be more of an example of the API needed to represent a stencil in Python, but I'm not sure whether this is useful anymore. There's a TODO comment to move to documentation, but something tells me it can happily not ever happen if no-one's working on the stencil project any more.

@superlopuh superlopuh added the testing PRs that modify tests label Feb 24, 2025
@superlopuh superlopuh self-assigned this Feb 24, 2025
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.25%. Comparing base (63b7198) to head (501892f).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3957      +/-   ##
==========================================
- Coverage   91.26%   91.25%   -0.01%     
==========================================
  Files         466      466              
  Lines       58054    58025      -29     
  Branches     5577     5577              
==========================================
- Hits        52982    52952      -30     
  Misses       3665     3665              
- Partials     1407     1408       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@georgebisbas georgebisbas left a comment

Choose a reason for hiding this comment

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

This was used as testing infra for the stencil dialect, and as an example. Overall it is tested enough I think. Are you removing def assert_print_op as well?

@superlopuh
Copy link
Member Author

Yeah that's the overall goal. It's the only helper that is shared between multiple tests, and is preventing us from testing xDSL as a library, meaning closer in behaviour to how people would use it when pip installing.

@superlopuh
Copy link
Member Author

@PapyChacal @AntonLydike @georgebisbas I'm happy to move this wherever you tell me to, but I'd like to delete this test, please let me know how to proceed.

@georgebisbas
Copy link
Contributor

Ι am happy with anything you do at this point here

@superlopuh
Copy link
Member Author

I'll take that as an approve :)

@superlopuh superlopuh merged commit c0de753 into main Feb 26, 2025
16 checks passed
@superlopuh superlopuh deleted the sasha/testing/remove-stencil-printing-test branch February 26, 2025 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing PRs that modify tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants