8000 internal: skip inconsistent test case by Young-Flash · Pull Request #2269 · moonbitlang/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

internal: skip inconsistent test case #2269

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 16, 2025
Merged

internal: skip inconsistent test case #2269

merged 2 commits into from
Jun 16, 2025

Conversation

Young-Flash
Copy link
Contributor
@Young-Flash Young-Flash commented Jun 13, 2025 •
edited by peter-jerry-ye
Loading

Skip some of the doc tests that behavior inconsistently across backends / OS, notably native backend on Windows.

@peter-jerry-ye-code-review peter-jerry-ye code review
Copy link
peter-jerry-ye-code-review bot commented Jun 13, 2025 •
edited
Loading
Inconsistent test documentation format in divison operations

Category
Maintainability
Code Snippet
test "panic Int64::op_div/division_by_zero" is split between two different code blocks
Recommendation
Keep related test cases in the same code block to maintain readability. Either move all division tests into one block or clearly separate normal cases from panic cases with clear comments.
Reasoning
Splitting related test cases across different documentation blocks makes it harder to understand the complete behavior of the division operation. Having them together provides better context and makes maintenance easier.

Duplicate test documentation across multiple files

Category
Maintainability
Code Snippet
Double::to_int64 test case appears identical in multiple files
Recommendation
Consider creating a shared test documentation that can be referenced across files, or clearly indicate why the duplicate tests are necessary for different implementations.
Reasoning
Duplicate test documentation can lead to maintenance issues if the expected behavior changes. Having a single source of truth for test cases makes updates more reliable.

Mixing of test attributes between 'moonbit' and 'skip'

Category
Maintainability
Code Snippet
Changes from moonbit to skip across multiple test cases
Recommendation
Document the reason for using 'skip' vs 'moonbit' in a comment at the top of each file or in the module documentation. Consider keeping a consistent approach for similar types of tests.
Reasoning
The inconsistent use of documentation attributes without clear reasoning makes it difficult for other developers to understand when to use which attribute. Clear guidelines would improve maintainability.

@coveralls
Copy link
Collaborator
coveralls commented Jun 13, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 7300

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.103%
Totals Coverage Status
Change from base Build 7296: 0.0%
Covered Lines: 8572
Relevant Lines: 9207
💛 - Coveralls

@Young-Flash Young-Flash force-pushed the skip_unconsistent_test branch 2 times, most recently from 61261d5 to 76c1120 Compare June 13, 2025 10:26
@Yu-zh Yu-zh changed the title internal: skip unconsistent test case internal: skip inconsistent test case Jun 14, 2025
@peter-jerry-ye peter-jerry-ye force-pushed the skip_unconsistent_test branch from 76c1120 to 9e36936 Compare June 16, 2025 02:07
@peter-jerry-ye peter-jerry-ye enabled auto-merge (rebase) June 16, 2025 02:07
@peter-jerry-ye
Copy link
Collaborator

cc @Kaida-Amethyst need further investigation.

@peter-jerry-ye peter-jerry-ye merged commit 2d65390 into main Jun 16, 2025
10 of 12 checks passed
@peter-jerry-ye peter-jerry-ye deleted the skip_unconsistent_test branch June 16, 2025 02:13
@peter-jerry-ye peter-jerry-ye mentioned this pull request Jun 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peter-jerry-ye peter-jerry-ye Awaiting requested review from peter-jerry-ye

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Young-Flash @coveralls @peter-jerry-ye
0