jsha / blocktogether

Share your blocks and subscribe to others'
GNU General Public License v3.0
330 stars 68 forks source link

new integration test: Blocking users with low followers #231

Closed dunn closed 7 years ago

dunn commented 7 years ago

The way I did this requires four new env variables:

The last two are so we can use the Twitter API directly to unblock the low-followers user after the tests, since doing it through the web UI via Casper didn't work.

jsha commented 7 years ago

Instead of ACCESS_TOKEN and ACCESS_SECRET in the env, how about pulling them from the DB after logging in, as in unblock-all.js? As-is, it's not clear which user the ACCESS_TOKEN belongs to, and it may get unwieldy to manage with more tests.

dunn commented 7 years ago

Oh, even better, we can just run unblock-all.js.

dunn commented 7 years ago

Oops, these passes are false positives: https://travis-ci.org/dunn/blocktogether/jobs/159382890#L1315

jsha commented 7 years ago

Overall, looks great - thanks for continuing to work on these tests!

dunn commented 7 years ago

Most of your comments are about doing things via the API rather than through Casper.

I forgot that at least some Node modules can be required in Phantom (http://docs.casperjs.orgf/en/latest/writing_modules.html), so I'll take a look at doing that.

jsha commented 7 years ago

If we can't pull in the modules directly we could probably shell out to executables, right? I.e. We can tweet by shelling out to jwurl with the right url (and possibly adding post support?)

On Sep 13, 2016 18:21, "Alex Dunn" notifications@github.com wrote:

Most of your comments are about doing things via the API rather than through Casper.

I forgot that at least some Node modules can be required in Phantom ( http://docs.casperjs.orgf/en/latest/writing_modules.html), so I'll take a look at doing that.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/jsha/blocktogether/pull/231#issuecomment-246877713, or mute the thread https://github.com/notifications/unsubscribe-auth/AANcLX_N8uZTQNf4nPwfZ1BqZLKRH9hhks5qp0wWgaJpZM4J6IVj .

dunn commented 7 years ago

I don't think so, since Casper can't do arbitrary code execution and the timing is important. But I'm pretty sure we'll find compatible modules in order to access the API.

dunn commented 7 years ago

For some reason the tests are acting up locally, so I might have to test with Travis for a bit; if you see a lot of small commits being pushed to this branch that's why.

dunn commented 7 years ago

Sorry for the delay; I'm looking at using the API functions and the Twitter clients are pulling in Node-specific stuff.

dunn commented 7 years ago

Now it's working locally and failing on Travis... (https://travis-ci.org/dunn/blocktogether/builds/160610253)

jsha commented 7 years ago

I'm going to go ahead and merge this - better to have the test than not, and we can work on improving it later. Thanks again!

jsha commented 7 years ago

Oh, I see you did move to using unblock-all to unblock through the API after all! Sorry, I had missed that on my last read through.

dunn commented 7 years ago

Yeah, I just couldn't figure out how to do the operations within the test via the API.