Add Unit Tests for Application and New CI Workflow for Vitest Coverage #1379
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| name: Vitest Coverage | ||
|
Member
There was a problem hiding this comment. What about a workflow that runs the tests on pull requests? I imagine we don't want to merge pull requests if they break our tests. |
||
|
|
||
| on: | ||
| release: | ||
| types: [published] | ||
| workflow_dispatch: | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 20 | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Run coverage | ||
| run: npm run coverage | ||
|
|
||
| - name: Zip coverage folder | ||
| run: zip -r coverage.zip ./coverage | ||
|
|
||
| - name: Upload coverage to release | ||
| uses: actions/upload-release-asset@v1 | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| with: | ||
| upload_url: ${{ github.event.release.upload_url }} | ||
| asset_path: ./coverage.zip | ||
| asset_name: coverage.zip | ||
| asset_content_type: application/zip | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,6 @@ | ||
| /node_modules | ||
| /build | ||
| .idea/ | ||
|
|
||
| /coverage | ||
| .vitest |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,31 @@ | ||
| import opencastConfig from "@opencast/eslint-config-ts-react"; | ||
|
|
||
| export default [ | ||
| ...opencastConfig, | ||
|
|
||
| // Fully ignore some files | ||
| { | ||
| ignores: ["build/"], | ||
| }, | ||
|
|
||
| { | ||
| plugins: { | ||
| vitest: await import("eslint-plugin-vitest"), | ||
| }, | ||
| languageOptions: { | ||
| globals: {}, | ||
| }, | ||
| rules: { | ||
| "arrow-parens": "off", | ||
| // TODO: We want to turn these on eventually | ||
| indent: "off", | ||
| "max-len": "off", | ||
| "no-tabs": "off", | ||
| "@typescript-eslint/no-explicit-any": "off", | ||
| "@typescript-eslint/no-floating-promises": "off", | ||
| "@typescript-eslint/no-misused-promises": "off", | ||
| "@typescript-eslint/no-unused-vars": "off", | ||
| "@typescript-eslint/require-await": "off", | ||
| }, | ||
| }, | ||
| ]; | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should we add code coverage reports to our releases? I feel like it will just unnecessarily bloat our releases with files that no one will ever look at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was my idea to give the topic of testing a bit more visibility. If we only create them as artifacts of e.g. each action run, I think nobody will ever look at them. This was just a first idea. I think there are other options as well. Maybe just publishing them using github.io might also be a possibility. Other ideas are also welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding test coverage to the releases is likely to get ignored, especially since we're aiming to automate away releases of the admin interface to Opencast as much as possible.
Another idea would be to add the coverage value to the Opencast release notes, somehow. Although I am not sure we want to proudly announce 5% code coverage in our release notes, so maybe not ^^'.
May be better if we could automatically post to github discussion, or even the matrix channel? No idea how to do that though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With all due respect, what I see here already represents nearly 20% of the entire application—a considerable portion of a clearly large codebase. I believe this level of coverage provides a solid starting point that could even be built on by myself.
This is simply my opinion, shared with the intention of contributing constructively.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not mean to belittle your work, this is definitely a large step up from our previous situation. Still, whether it is 5% or 20%, neither of these numbers is nearly large enough to inspire confidence in our adopters (and especially new adopters). Instead, I fear that it would have quite the opposite effect. Therefore, I think we should consider carefully how we want to draw attention to this.