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

ci: native backend test on windows #1116

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

Young-Flash
Copy link
Contributor

No description provided.

Copy link

peter-jerry-ye-code-review bot commented Oct 12, 2024

Based on the provided git diff output, here are three observations and suggestions for potential issues:

  1. Unusual ulimit Setting in GitHub Workflow:

    - name: moon test in native backend (unix)
      if: ${{ matrix.os != 'windows-latest' }}
      run: |
        ulimit -s 8176
        moon test --target native
        moon test --target native --release

    Suggestion: The ulimit -s 8176 command sets the stack size limit to 8176 KB, which is quite specific. Ensure that this value is necessary and appropriate for the tests being run. If not, consider reverting to the default or a more standard value to avoid potential issues with memory usage.

  2. Inconsistent Release Support for Windows in GitHub Workflow:

    - name: moon test in native backend (windows)
      if: ${{ matrix.os == 'windows-latest' }}
      run: |
        # --release not support for windows
        moon test --target native --dry-run
        moon test --target native

    Suggestion: The comment # --release not support for windows suggests that running tests in release mode is not supported on Windows. This inconsistency could lead to differences in behavior between platforms. Consider investigating why release mode is not supported on Windows and either address the issue or document it clearly to avoid confusion.

  3. Modification to moon.mod.json:

    {
      "readme": "README.md",
      "repository": "https://github.com/moonbitlang/core",
      "license": "Apache-2.0",
      "keywords": ["core","standard library"],
      "link-flags": ["-cc", "cl /nologo"]
    }

    Suggestion: The addition of "link-flags": ["-cc", "cl /nologo"] in moon.mod.json indicates a change in compilation settings. Ensure that these flags are necessary and correctly configured for the build process. Specifically, verify that cl /nologo is appropriate for the intended compiler and that it does not introduce any unintended side effects or compatibility issues.

These suggestions aim to address potential issues related to resource management, platform consistency, and build configuration, ensuring a smoother development and testing workflow.

@coveralls
Copy link
Collaborator

coveralls commented Oct 12, 2024

Pull Request Test Coverage Report for Build 3586

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 82.319%

Totals Coverage Status
Change from base Build 3580: 0.0%
Covered Lines: 4195
Relevant Lines: 5096

💛 - Coveralls

@Young-Flash Young-Flash force-pushed the native_backend_test_ci branch 7 times, most recently from 8259ab9 to fd37fdd Compare October 15, 2024 08:20
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