-
Notifications
You must be signed in to change notification settings - Fork 8
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
Include test coverage results in GitHub Action CI script #1148
Comments
|
Instructions look good; learning about secrets systems comes with the territory so is a good aspect of these tickets |
The next steps for @akaiap are listed above, plus she will verify the preferred location for the secret |
@akaiap reported on the actions she took leading to the comment above; it looks like some setup questions need to be resolved:
|
I met with @dondi today and successfully generated the token from Coveralls.io and placed it in GRNsights secrets. However, I ran into a couple of problems when building. (Very close!)
![]() Coveralls Error Summary:
![]() During our meeting, we can relink the github to coveralls.io! |
Interesting development—
Key Takeaways:
During our meeting we should still try to relink GRNsight with Coveralls.io |
I tested the build again after moving the chai.should() call below all the import statements. While the build still ran, it triggered warnings and linting errors.
Possible solutions:CommonJS vs. ES ModulesOption 1: Convert the Project to Full ES Modules
Option 2: Use a Hybrid Approach (Conditional Imports) (supporting both esm and commonjs) Fixing ESLint IssuesOption 1: Update eslint.config.js to Match ESLint 9 Standards
Option 2: Downgrade ESLint
Version UpdatesUpdates within NodeJs:
Package-lock.json Changes
|
For the issues documented above, we’ve determined the following next steps:
|
Here's a summary of my findings and proposed solutions:1. ESLint Configuration and Linting ErrorsMigration to ESLint version 9 has caussed challenges due to changes in its config structure:
Proposed Solutions:
2. chai.should() Integration Issues
Proposed Solutions:
an example of what the dynamic approach will look like-- will test to verify.
3. Documentation UpdatesI have begun drafting updates to the wiki, particularly focusing on the recent changes to thq library dependencies and configuration files. Once the linting issues are fully resolved, I will push these updates to the wiki. Next Steps:
|
@akaiap will separate this ticket into at least a documentation issue and configuration/Chai issues, or possibly three distinct ones After concluding research, go ahead update the PR as needed so we can go back to a review phase |
In addition to running tests, we should start recapturing coverage results from the tests (formerly handled by Coveralls) and include this in the script as well. Per #1146, once we have coverage captured, it would be good to also include these results in the README badge
The text was updated successfully, but these errors were encountered: