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

Jest testing & test for add step #1366

Merged
merged 7 commits into from
Dec 18, 2019
Merged

Jest testing & test for add step #1366

merged 7 commits into from
Dec 18, 2019

Conversation

keshav234156
Copy link
Member

Fixes #1242

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/is-reviewers for help, in a comment below
  • Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

@keshav234156
Copy link
Member Author

cc: @jywarren @harshkhandeparkar

@keshav234156
Copy link
Member Author

keshav234156 commented Dec 17, 2019

run it with npm run test-ui-2

@codecov
Copy link

codecov bot commented Dec 17, 2019

Codecov Report

Merging #1366 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1366   +/-   ##
=======================================
  Coverage   66.23%   66.23%           
=======================================
  Files         125      125           
  Lines        2559     2559           
  Branches      400      400           
=======================================
  Hits         1695     1695           
  Misses        864      864

@jywarren
Copy link
Member

This is awesome. Following eagerly! Please ping me once the path is local. Great work! cc @harshkhandeparkar @vaibhavmatta @tech4GT and others! ♥️😁🎉

@jywarren
Copy link
Member

Could you add the run command to the overall test trigger so they run in Travis?

@keshav234156
Copy link
Member Author

@jywarren Do you know how can we make relative to examples/index.html ?

@keshav234156
Copy link
Member Author

ohk done with fs library as suggested by @harshkhandeparkar

@keshav234156
Copy link
Member Author

cc: @jywarren
Screenshot from 2019-12-18 04-21-19

https://travis-ci.org/publiclab/image-sequencer/jobs/626434689#L1683

Horray!!!
We have now test running on Travis as well

@keshav234156
Copy link
Member Author

@jywarren Now we just need to add it to npm test-ui command properly . How can we do that?

@jywarren
Copy link
Member

This is Awesome!!! Can you try breaking a test to be sure it triggers a Travis failure? 🎉🎉🎉

@keshav234156
Copy link
Member Author

let's check with title

Copy link
Member

@jywarren jywarren left a comment

Choose a reason for hiding this comment

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

Great! Ok now we add it here;

- npm run test-ui

package.json Outdated Show resolved Hide resolved
@keshav234156
Copy link
Member Author

keshav234156 commented Dec 17, 2019

@jywarren travis failed as i changed title of page

https://travis-ci.org/publiclab/image-sequencer/jobs/626440796#L1709

Copy link
Member

@jywarren jywarren left a comment

Choose a reason for hiding this comment

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

Once we've merged, we can write an issue with a longer list of critical UI functions we want to test, and start getting gci folks to pitch in maybe!

src/test/frontend.test.js Outdated Show resolved Hide resolved
@jywarren
Copy link
Member

Let's just remove a few of the console.log statements too and add a brief readme entry? Thanks, this is amazing!!!!!

@keshav234156 keshav234156 changed the title [WIP] test for add step test for add step Dec 17, 2019
@keshav234156
Copy link
Member Author

@jywarren LGTM

@jywarren
Copy link
Member

Ah, I see a merge conflict, can you rebase? Then good to go!!!!! Thanks!

@keshav234156
Copy link
Member Author

keshav234156 commented Dec 17, 2019

@jywarren in which file?

@jywarren
Copy link
Member

jywarren commented Dec 18, 2019 via email

@keshav234156
Copy link
Member Author

keshav234156 commented Dec 18, 2019

@jywarren where it is shown in Github whether Pr is having conflicts
Screenshot from 2019-12-18 05-47-02

Though i have updated the branch.

Are merge conflicts still present

sorry for asking stupid questions here.I have very lot of problems with git

@jywarren
Copy link
Member

Ah! I dunno what happened, I saw a message earlier but now it's just asking for 2 reviews. I can bypass but let's aim for 2 in future PRs! Thanks!

@jywarren jywarren merged commit ed0f76c into publiclab:main Dec 18, 2019
@jywarren
Copy link
Member

🎉🎉🎉🎉 awesome. Would you have interest in the list of tests issue and/or the readme update? This is so great!!!!!

@keshav234156
Copy link
Member Author

Ok will make list of ui test nedded.some of them are already listed in #1000

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.

None yet

2 participants