advweb-grp1 / advanced-web-final-year-project

Final year advanced web develop unit project
MIT License
1 stars 0 forks source link

Core/workflow testing #23

Closed LiamSingh64 closed 1 year ago

LiamSingh64 commented 1 year ago

This PR changed the import path for vite.config.js in the vitest.config.js file so that the commands npm run test:unit and npm run test:e2e work correctly and pass.

These commands should also be added to the Action Workflows which worked when ran locally. I suggest you switch to the 'core/workflow-testing' branch, and run these commands locally as well to make make sure they work/pass for you.

Please comment any issues/thoughts you find with these changes.

github-actions[bot] commented 1 year ago

Visit the preview URL for this PR (updated for commit d43f349):

https://adv-web-grp1--pr23-core-workflow-testin-vbys3bbn.web.app

(expires Mon, 10 Apr 2023 20:48:33 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: e45f8bd17b7de44787580bea572e36aa09784b8c

rwx-yxu commented 1 year ago

Eslint merge completed. Please can you double check if npm run ci will run linting for us. If not, then we will need to alter the workflow file to add npm run lint. Merging in main will be required for the eslint config setup first to test out.

LiamSingh64 commented 1 year ago

Build is failing mainly bcos some components have SVG's with 1 really long string that is exceeding ESLints 'max-len' rule (120 characters are only allowed on 1 line of code).

Should we remove this rule? Extend the number of characters allowed to fit the components? Remove these components? Find a way for Lint to split strings that exceed limit?

advweb-grp1 commented 1 year ago

Yeet the icon files. Modify eslint config such that it now will exclude .vite.config.js and .cy* files because the tests are able to be ran just fine.

LiamSingh64 commented 1 year ago

Ok so I deleted all the Icon .vue files but "src/components/TheWelcome.vue" imports all of them, and since we're not using this aswell, I just deleted it.

LiamSingh64 commented 1 year ago

the flag --ignore-path for ESLint doesn't allow multiple paths so a workaround I found is to add an Environment comment which works totally fine!:thumbsup:

running npm run lint worked and passed everything when I ran it with these changes

advweb-grp1 commented 1 year ago

Build failed because the delete component was referenced in the home view.vue file. This shouldn't be handled in this pull request tbh because it dilutes what this pull request is doing when the original intention for this pull request is to import path for vite.config.js in the vitest.config.js file so that the commands npm run test:unit and npm run test:e2e work correctly and pass. I am going to remove the references to it and make deleted component and make the home view file just output "This is the home page."

rwx-yxu commented 1 year ago

Resolving the linting import issues by adding some cypress config options to eslintrc.js file and disabling linting for import { configDefaults } from 'vitest/config'; no idea why eslint isn't able to resolve this import but I can not seem to find a good solution online. Most sources point to adding this setting into the eslint.js file

  settings: {
    'import/resolver': {
      alias: {
        map: [
          ['vitest/config', 'vitest/dist/config.js'],
        ],
        extensions: ['.js', '.vue'],
      },
    },
  },

But that doesn't work. I have verified that I can manually the test files on local