Fix cycle in dependencies to get tests running

quicksilver / Quicksilver

Quicksilver Project Source

http://qsapp.com

Apache License 2.0

2.72k stars 283 forks source link

Fix cycle in dependencies to get tests running #2916

Closed n8henrie closed 10 months ago

n8henrie commented 1 year ago

Revert "Disable parellalize builds for preflight"
Fix comment prefix
Remove multiple deps from Quicksilver Tests to fix cycle in dependencies

n8henrie commented 1 year ago

Removed several dependencies from Quicksilver Tests and they seem to at least run now:

pjrobertson commented 1 year ago

The tests on CI are still failing. See: https://github.com/quicksilver/Quicksilver/runs/7736472992?check_suite_focus=true#step:3:33

Is there a reason you reverted "Revert "Disable parellalize builds for preflight"? I'm wondering if that's the problem

I tested this on my local Mac though, and can confirm things still build.

n8henrie commented 1 year ago

The tests on CI are still failing.

Quick push before had to run for dinner. I'll keep iterating on it -- but for the first time (ever) the tests are actually running on my machine, so I'm excited. There are several test failures -- which also makes me excited.

Is there a reason you reverted "Revert "Disable parellalize builds for preflight"?

Because it didn't seem to change anything on my end, and I'd rather isolate the problem rather than change multiple variables, and I'd rather have faster builds if it ends up not being an issue.

I'm wondering if that's the problem

I don't think so -- in fact I doubt it even has an any effect on newer machines (since it's not a user-facing setting anymore).

pjrobertson commented 1 year ago

Sure thing, as long as we can get the tests running successfully on CI, then I'm happy!

FYI: the 3 tests you had that failed in QSFileObjectTests all pass for me. So it may just be a naming convention on newer Macs. Here's mine:

Screen Region 2022-08-09 at 09 01 49

Also - I pushed two more commits here since I added my own unit test, and thought it'd be easier to add it here than to master :)

n8henrie commented 1 year ago

Sure thing, as long as we can get the tests running successfully on CI, then I'm happy!

Same here! Running or appropriately failing (as opposed to just not running).

FYI: the 3 tests you had that failed in QSFileObjectTests all pass for me. So it may just be a naming convention on newer Macs.

For testFileNaming, the (helpful) comment suggests perhaps a locale issue? // label is localized, so this test might only work for a specific locale

(lldb) po [object name]
Safari.app
(lldb) po [object label]
 nil

pjrobertson commented 10 months ago

All these tests pass for me (except for one). Amazing! I'm going to merge this :)

This one test fails, I think it's because I'm running unsigned. Should probably work on signed.