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

get file type tests working again #1698

Merged
merged 1 commit into from Dec 3, 2013
Merged

get file type tests working again #1698

merged 1 commit into from Dec 3, 2013

Conversation

skurfer
Copy link
Member

@skurfer skurfer commented Dec 2, 2013

…now that they’re being looked at by something other than a human that can say “Yeah, yeah, I know”.

@tiennou
Copy link
Member

@tiennou tiennou commented Dec 2, 2013

Hint: I'll likely move testing out to something that supports mocks (like specta & expecta), so I don't think it's necessary to fix those.

@skurfer
Copy link
Member Author

@skurfer skurfer commented Dec 3, 2013

We’re obviously not motivated to add tests as it is. Do we really want to add another dependency to defining them?

@tiennou
Copy link
Member

@tiennou tiennou commented Dec 3, 2013

Yes, because not testing a project of that size is unacceptable, OCTest is woefully underpowered for that task, Specta's syntax is less verbose, and I'm planning to rewrite everything so I'll write tests.

But I didn't want to stop you from fixing them anyway, it's just that I'll rewrite them as soon as I get near QSObject and friends ;-).

@pjrobertson
Copy link
Member

@pjrobertson pjrobertson commented Dec 3, 2013

No harm in merging this whichever way we go with testing.

Since using unit testing in some of my own projects, it's totally something worth doing. But starting off for a project like Quicksilver (messy, and writing tests after the code has been written) just isn't fun. Personally I don't have that much motivation to write tests :(

pjrobertson added a commit that referenced this issue Dec 3, 2013
get file type tests working again
@pjrobertson pjrobertson merged commit 9e59835 into master Dec 3, 2013
1 check passed
@pjrobertson pjrobertson deleted the fileTypeTests branch Dec 3, 2013
@skurfer
Copy link
Member Author

@skurfer skurfer commented Dec 3, 2013

Yes, because not testing a project of that size is unacceptable, OCTest is woefully underpowered for that task, Specta's syntax is less verbose, and I'm planning to rewrite everything so I'll write tests.

If you’re willing to commit to Specta by writing tests for the classes you’re re-working, then I have no objections. Ideally, the tests would come before the rewrite, but it’s a big mess anyway, so handle that however you want. 😃

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Dec 3, 2013

Great, now I don't have failing tests anymore, to test if jenkins acts correctly if there are failing tests. :-/
Oh, well see the next time you break it. ;-)

@skurfer
Copy link
Member Author

@skurfer skurfer commented Dec 3, 2013

Great, now I don't have failing tests anymore, to test if jenkins acts correctly if there are failing tests.

It’s only fixed on master. There should be numerous other branches out there where it’s still broken. :-)

@HenningJ
Copy link
Contributor

@HenningJ HenningJ commented Dec 3, 2013

It’s only fixed on master. There should be numerous other branches out there where it’s still broken. :-)

Of course you're right. Good point. Works as I hoped.

But while doing that, I noticed something else: when does the .dmg get created? When I first tried it a few weeks ago, it was created every time Release was built (right?). Did that change recently? Because now I actually clean the /tmp/QS/ dir before each built and now I never get a .dmg file. What changed?

@skurfer
Copy link
Member Author

@skurfer skurfer commented Dec 4, 2013

The DMG is created by Tools/qsrelease, which is new. See #1658

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

4 participants