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

(chore) : Added Headless Testing #733

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Abhijay007
Copy link
Contributor

@Abhijay007 Abhijay007 commented Jan 20, 2023

Signed-off-by: Abhijay Jain Abhijay007j@gmail.com

refer PR: #641 (resolved merge conflicts in existing PR)

  • Added mocha-chrome as a grunt task, which is borrowed from p5.js.
  • Changed the test filing architecture and the initialization of expect in all test files so that all the files go to one spec array.
  • Added karma-js as it seems more consistent with tests.
  • We currently, cannot use it in GitHub automation as it is still not 100 % reliable. Some tests are still failing in some cases.
  • We can add retry functionality to some tests which can be identified as failing and maybe one day in the future, we can add it to GitHub actions.

Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
@Abhijay007 Abhijay007 marked this pull request as draft January 20, 2023 18:38
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
@Abhijay007
Copy link
Contributor Author

cc : @davepagurek , @satyasaibhushan review required.

@Abhijay007 Abhijay007 marked this pull request as ready for review January 26, 2023 16:36
@davepagurek
Copy link
Contributor

Thanks for taking on this PR!

When I try to run npm install to get the new dependencies, I run into this error:
image

Is this also something you get or is it just me?

@davepagurek
Copy link
Contributor

We currently, cannot use it in GitHub automation as it is still not 100 % reliable. Some tests are still failing in some cases.

Out of curiosity, how many test cases are failing? If it's a small enough amount, a great follow-up PR could comment them out for now, so that we get automated PR testing for most things, and then we can make some issues to fix the lingering tests. If it's a lot though, it sounds like we might need to solve some underlying issues first

Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
@Abhijay007
Copy link
Contributor Author

Abhijay007 commented Jan 28, 2023

Thanks for taking on this PR!

When I try to run npm install to get the new dependencies, I run into this error: image

Is this also something you get or is it just me?

I faced similar issues while installing these packages, I uninstalled the mentioned packages and re-installed them using --force, maybe it is due to version incompatibly, I am not sure about it but will look into this :)

@Abhijay007
Copy link
Contributor Author

We currently, cannot use it in GitHub automation as it is still not 100 % reliable. Some tests are still failing in some cases.

Out of curiosity, how many test cases are failing? If it's a small enough amount, a great follow-up PR could comment them out for now, so that we get automated PR testing for most things, and then we can make some issues to fix the lingering tests. If it's a lot though, it sounds like we might need to solve some underlying issues first

@davepagurek 20 tests are falling in total :

image

@Abhijay007
Copy link
Contributor Author

We currently, cannot use it in GitHub automation as it is still not 100 % reliable. Some tests are still failing in some cases.

Out of curiosity, how many test cases are failing? If it's a small enough amount, a great follow-up PR could comment them out for now, so that we get automated PR testing for most things, and then we can make some issues to fix the lingering tests. If it's a lot though, it sounds like we might need to solve some underlying issues first

@davepagurek 20 tests are falling in total :

image

I will open a follow-up PR once we merge this one, commenting all those falling tests

Signed-off-by: Abhijay Jain <Abhijay007j@gmail.com>
@satyasaibhushan
Copy link
Collaborator

@Abhijay007
Why did you open a new PR when you can add the changes needed to this old PR?
Doing it this way will remove my original commits.

Copy link
Collaborator

@satyasaibhushan satyasaibhushan left a comment

Choose a reason for hiding this comment

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

If you really want to open a new PR, you can cherry-pick the commits done my be in the original PR.

@Abhijay007
Copy link
Contributor Author

@Abhijay007 Why did you open a new PR when you can add the changes needed to this old PR? Doing it this way will remove my original commits.

@satyasaibhushan I didn't have the access to resolve the conflicts directly that's why I opened a new PR resolving the existing one, and I was not aware of the cherry-pick commits technique before, thanks for mentioning that, I will iterate the PR accordingly.

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.

3 participants