adafruit / adabot

Adabot is our robot friend who helps Adafruit online
MIT License
13 stars 27 forks source link

Add More Tests #237

Closed sommersoft closed 3 years ago

sommersoft commented 3 years ago

Adds more tests for the following:

sommersoft commented 3 years ago

Please don't merge this yet. The update_cp_org_libraries tests are taking an unreasonable amount of time in Actions. 1 hour in Actions vs ~25 seconds locally. I'm going to run some research on my fork.

evaherrada commented 3 years ago

@sommersoft As in running update_cp_org_libraries.py or just the actions for it?

sommersoft commented 3 years ago

@sommersoft As in running update_cp_org_libraries.py or just the actions for it?

Specifically just the test for update_cp_org_libraries.py that I added here. Once it gets to this test, it takes an hour.

Locally, the entire test suite takes ~25 seconds. On my fork, the entire test suite takes ~45 seconds (exact copy of this branch here).

The long runtime may just be a one-off in this PR, having to do with the Actions container/cache/etc. I'm still trying to narrow down my confidence level on that though.

evaherrada commented 3 years ago

@sommersoft Ah, I see. Oh and "libraries" is spelled "libraires" in a few tests.

evaherrada commented 3 years ago

(looks like that autocorrected to the correct spelling, I replaced it with the correct incorrect spelling)

sommersoft commented 3 years ago

@sommersoft Ah, I see. Oh and "libraries" is spelled "libraires" in a few tests.

I can't see anything in the code, nor in the repo differences, that would be causing the long runtime. So, maybe we use the typos as a testing point.

We can merge this PR as-is, and I'll submit another PR with the typo corrections. Then we can see if the long runtime re-occurs or if it was a one-off.

Thoughts?

evaherrada commented 3 years ago

@sommersoft Sure. Let me review it real quick.

sommersoft commented 3 years ago

Haha, the tests ran in 48s on the merge/push action trigger. Hopefully that answers that. 😄

I'll have the typos PR in shortly.