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

Merged
merged 1 commit into from
Dec 9, 2024
Merged

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 4146

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

Totals Coverage Status
Change from base Build 4142: 0.0%
Covered Lines: 4493
Relevant Lines: 5558

💛 - Coveralls

@Young-Flash Young-Flash force-pushed the native_backend_test_ci branch 8 times, most recently from fd37fdd to f1838e4 Compare October 16, 2024 07:29
@bobzhang
Copy link
Contributor

bobzhang commented Dec 8, 2024

@Young-Flash is this PR still relevant?

@Young-Flash Young-Flash force-pushed the native_backend_test_ci branch from f1838e4 to bf7c210 Compare December 9, 2024 03:16
@Young-Flash Young-Flash marked this pull request as ready for review December 9, 2024 03:19
@Young-Flash Young-Flash merged commit f60be25 into main Dec 9, 2024
13 checks passed
@Young-Flash Young-Flash deleted the native_backend_test_ci branch December 9, 2024 03:19
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.

3 participants