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

Adds initial CI pipeline support #13

Merged
merged 2 commits into from
Nov 21, 2024
Merged

Adds initial CI pipeline support #13

merged 2 commits into from
Nov 21, 2024

Conversation

dylon
Copy link
Collaborator

@dylon dylon commented Nov 20, 2024

Major changes:

  • Adds pipeline stages for running the unit tests on Ubuntu Linux, MacOS, and Windows.
  • Adds pipeline stage for linting the sources.
  • Cleans up existing sources according to the linter's recommendations.
  • Replaces the test script for stubbing lfortran responses with sinon stubs for Windows compatibility (I could not get the script to run in the unit tests on Windows, and this is a better approach anyway).
  • Updates dependencies since most of them were at least 2 years old.
  • Adds CI pipeline status badges to README.

…Windows; adds CI pipeline stage to lint sources; cleans up sources according to recommendations from linter; replaces the lfortran testing script with sinon stubs; adds CI pipeline badges to readme
@certik
Copy link
Contributor

certik commented Nov 20, 2024

The CI has the following failure in npm test:

TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined
    at Object.writeFileSync (node:fs:2354:5)
    at LFortranCLIAccessor.runCompiler (D:\a\lfortran-lsp\lfortran-lsp\server\src\lfortran-accessors.ts:103:10)
    at LFortranCLIAccessor.showErrors (D:\a\lfortran-lsp\lfortran-lsp\server\src\lfortran-accessors.ts:241:33)
    at LFortranLanguageServer.validateTextDocument (D:\a\lfortran-lsp\lfortran-lsp\server\src\lfortran-language-server.ts:295:27)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5) {
  code: 'ERR_INVALID_ARG_TYPE'
}

But the test "passes". Is that expected?


How is LFortran installed at the CI? I can't find the installation process, but the tests seem to use it?

@dylon
Copy link
Collaborator Author

dylon commented Nov 21, 2024

The CI has the following failure in npm test:

TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined
    at Object.writeFileSync (node:fs:2354:5)
    at LFortranCLIAccessor.runCompiler (D:\a\lfortran-lsp\lfortran-lsp\server\src\lfortran-accessors.ts:103:10)
    at LFortranCLIAccessor.showErrors (D:\a\lfortran-lsp\lfortran-lsp\server\src\lfortran-accessors.ts:241:33)
    at LFortranLanguageServer.validateTextDocument (D:\a\lfortran-lsp\lfortran-lsp\server\src\lfortran-language-server.ts:295:27)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5) {
  code: 'ERR_INVALID_ARG_TYPE'
}

But the test "passes". Is that expected?

How is LFortran installed at the CI? I can't find the installation process, but the tests seem to use it?

The error is expected. The tests with errors check how well the server recovers from erroneous output from lfortran. mocha does not swallow stdout or stderr when it runs the tests, so all the logging information gets printed in the tests.

lfortran is not installed as part of the unit tests. I need to add an additional suite of integration tests for it and VSCode. The unit tests are intended to assert that our extension behaves as intended with the expected outputs from lfortran and the expected inputs from VSCode. The unit tests should not break if something changes in lfortran or VSCode, but the integration tests should. The integration tests will also be much slower to initialize and run.

Edit: I'll work on adding the integration tests for the rest of today. I didn't want this PR to get too big so I went ahead and submitted it.

Edit: The call to lfortran is stubbed with sinon in the unit tests. You'll see statements like sinon.stub(lfortran, "runCompiler").resolves(stdout). Those override the output from calling lfortran.runCompiler(...) to return whatever is the value of stdout. lfortran is not actually called from the unit tests.

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

You can merge it. See #14 for adding lfortran.

@dylon dylon merged commit 8989d9a into main Nov 21, 2024
4 checks passed
@dylon dylon deleted the dylon/ci branch November 21, 2024 00:21
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