Sherlock: Add a CI verification

Created on 21 Mar 2020  ·  22Comments  ·  Source: sherlock-project/sherlock

Such as Github Actions. If you guys let me, I can send a PR

enhancement

Most helpful comment

OK, GitHub actions it is. It is probably a better long term direction anyway, as it will integrate better with GitHub.

@luchiago if you have the time, a PR would be welcome. I am going to look at it a bit myself, but I am not confident I will be able to get very far.

One thing that would really be nice is a nightly run against all of the sites (with an easy to scan report about errors). Too often a site changes their behavior, and Sherlock starts giving false positives/negatives. Only a few attentive users actually notice the problem.

All 22 comments

We had this turned on at one time, but somehow it got shut off.

@TheYahya or @sdushantha do you have the ability to turn Travis CI back on? Or, do we want to go with GitHub Actions?

I noticed that GitHub Actions has automation for deploying Python packages to PyPi. Once when the packaging activity is done, this would be handy to simply deploy. So, maybe it would be good to investigate GitHub Actions instead of Travis CI?

@hoadlck I am pretty sure @TheYahya has the ability to turn Travis CI on because I never enabled it myself. GitHub Actions seems to very nice, many projects use it. I have limited knowledge of it, but I have used in the past and it worked well.

I got Insufficient permission error

If that is the case, then I guess we can move to GitHub Actions? 🤷‍♂️

OK, GitHub actions it is. It is probably a better long term direction anyway, as it will integrate better with GitHub.

@luchiago if you have the time, a PR would be welcome. I am going to look at it a bit myself, but I am not confident I will be able to get very far.

One thing that would really be nice is a nightly run against all of the sites (with an easy to scan report about errors). Too often a site changes their behavior, and Sherlock starts giving false positives/negatives. Only a few attentive users actually notice the problem.

I created this file that does the same types of things that the Travis file did.

EDIT: It did not run successfully. But, it did run and fail right away. So...yay?

Okay, @hoadlck. Thanks for allowing me to work on this issue. I'll do my search and soon I will open a PR.

I updated the file I added earlier so that it does execute the basic detect tests. But, I am unsure if this will run the tests on pull requests automatically. And, I did not create a nightly test that does the coverage tests.

Pull requests should probably run the full check of all of the sites. The command for that is
cd sherlock && python -m unittest tests.all --verbose.

But, does it really need to run this under all languages?

@hoadlck My idea was just verified if the project can make a build in the Github actions. I think that should run all the tests when the change is on the master branch.

But what about running the tests on a pull request before it is merged?

I couldn't find (yet) a way to run all the tests without taking too much time, bc there are many, right?

Running the full tests on my PC takes 428.825s. The basic tests only takes 3.270s. But, that is just for one version on Python.

Could we have a tests on PRs that only ran under one version of Python?

Which version? And why exists this difference?

I am just wondering what the options are. I am not familiar with GitHub Actions, so I wonder what kind of flexibility exists.

If the tests for all 3 Python versions take 21 minutes, and doing just one takes 7 minutes, is it worth it to always run all 3?

If someone just adds a new site (i.e. just changes the data.json file), there is no value in running the tests against all of the Python versions. However, if there was an algorithmic change, then we would want to. Can the tests do the right thing for either of these situations?

Hmm now I did understand what you're looking for. I don't know exactly if this can be done, but I'll do my research.

Now it's ready for review @hoadlck :smile:

After I merged #607 to master, the test ran and I got a failure.

It looks like the problem is when the tests run on master, the following line fails:

URL="https://api.github.com/repos/sherlock-project/sherlock/pulls/${{ github.event.pull_request.number }}/files"

I guess it makes sense: there is no pull request when the feature is on master.

I am leaving the development included for now, as it does work well for pull requests. But, I am reopening this issue to track the need to fix the test when it runs on master.

I will search about it @hoadlck sorry I didn't pay attention to this case.

As I look at this more, I am wondering if we should just have a separate actions for pull requests and for the master branch?

As it is, the badge that shows the state of the tests will have run a different process depending on what the last change was. Which does not seem quite right.

If instead we had a unique action for the pull requests, then it would be simpler to manage.

I think it's a good idea, create two separate github actions config files, one for master another for pull requests. The badge will be from the github action for the master and will run all the python versions.

Yo man much appreciated with the extra ... I just started so I’m still tryna find out how to get python and how to even make the code run smoothly I’m learning everything on my own smh if you have any advice for a new Comer let me know l

Sent from my iPhone

On May 19, 2020, at 7:15 PM, Lucas Hiago notifications@github.com wrote:


I think it's a good idea, create two separate github actions config files, one for master another for pull requests. The badge will be from the github action for the master and will run all the python versions.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

termashacker69 picture termashacker69  ·  6Comments

xch1029 picture xch1029  ·  6Comments

nareddyt picture nareddyt  ·  4Comments

manishyadav1400 picture manishyadav1400  ·  7Comments

tdh8316 picture tdh8316  ·  3Comments