-
Notifications
You must be signed in to change notification settings - Fork 140
Set up Portfolio Functional Tests #2721
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
Set up Portfolio Functional Tests #2721
Conversation
Update portfolio functional tests and also update the portfolio test snapshots.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2721 +/- ##
========================================
Coverage 52.84% 52.84%
========================================
Files 130 130
Lines 7162 7162
Branches 1594 1503 -91
========================================
Hits 3785 3785
+ Misses 3222 3072 -150
- Partials 155 305 +150 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR updates the portfolio functional tests by updating the snapshot files to reflect the new MarkBind 6.0.0 release and by adding the portfolio template to the testSites list. The key changes include:
- Renaming a project key in the expected siteData.json.
- Upgrading snapshot files (HTML, JS, and other assets) to MarkBind 6.0.0 while removing obsolete files.
- Adding the portfolio template reference in testSites.js.
Reviewed Changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/test/functional/test_site_templates/test_portfolio/expected/siteData.json | Renamed the project key from "example-project-ip-for-cs2103" to "project-1" for clarity. |
| packages/cli/test/functional/test_site_templates/test_portfolio/expected/index.html | Updated meta generator tag, script load order and content reflecting MarkBind 6.0.0 changes. |
| packages/cli/test/functional/test_site_templates/test_portfolio/expected/index.page-vue-render.js | Replaced the old Vue render function with the new compiled render function via a dynamic Function. |
| packages/cli/test/functional/test_site_templates/test_portfolio/expected/* | Removed several outdated snapshot files (e.g. index.md, skills.md, projects.md, etc.) which seem to be consolidated in the new snapshot design. |
| packages/cli/test/functional/testSites.js | Added the portfolio test template to the list of test sites. |
Comments suppressed due to low confidence (2)
packages/cli/test/functional/test_site_templates/test_portfolio/expected/siteData.json:12
- Please confirm that renaming the project key from 'example-project-ip-for-cs2103' to 'project-1' is intentional and consistent with the naming conventions used across portfolio snapshots.
"project-1": "Example project: iP for CS2103",
packages/cli/test/functional/test_site_templates/test_portfolio/expected/index.html:7
- Verify that the updated generator meta tag and the revised script loading order for MarkBind 6.0.0 are aligned with the latest documentation and provide the expected functionality in the portfolio tests.
<meta name="generator" content="MarkBind 6.0.0">
AgentHagu
left a comment
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.
LGTM! The tests are running and passing as expected!
|
Need to update to account for ignore of render js files |
What is the purpose of this pull request?
Overview of changes:
Update portfolio functional tests and also update
the portfolio test snapshots.
See #2720 for details.
Fixes #2720
NOTE:
Note that this PR #2721 will have merge conflicts with PR #2718. This is because of the
page-vue-render.jsfiles that are being checked and commited. After merging one, I will resolve the merge conflicts as needed for the other. It does not matter which PR is merged first.It also looks like there are many files changed, but I only did two things:
PR-reviewer would just have verify that any changes are due to these two steps, and there are no unrelated changes or regressions.
Anything you'd like to highlight/discuss:
I believe the files for the functional tests for portfolio may have been copied over manually previously when it was added, causing the setup of the functional tests for the portfolio template to be overlooked. This PR rectifies this.
Testing instructions:
npm run testin the packages/cli directory.Ensure portfolio snapshot functional tests run, and pass.
Proposed commit message: (wrap lines at 72 characters)
Set up Portfolio Functional Tests
Update portfolio functional tests and update
portfolio test snapshots through updatetest.
Checklist: ☑️
Reviewer checklist:
Indicate the SEMVER impact of the PR:
At the end of the review, please label the PR with the appropriate label:
r.Major,r.Minor,r.Patch.Breaking change release note preparation (if applicable):