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

Use TARGET_FILE in ctlrender unittest. Fixes #45. #46

Merged
merged 1 commit into from
May 27, 2014

Conversation

cbenhagen
Copy link
Contributor

No description provided.

@aforsythe
Copy link
Member

Ben.

Thanks. I'll test shortly and I all goes well merge.

Alex

On May 26, 2014, at 3:56 PM, "Ben Hagen" <notifications@github.commailto:notifications@github.com> wrote:


You can merge this Pull Request by running

git pull https://github.com/cbenhagen/CTL fix#45

Or view, comment on, or merge it at:

#46

Commit Summary

File Changes

Patch Links:


Reply to this email directly or view it on GitHubhttps://github.com//pull/46.

[Δ]http://www.oscars.org

Alex Forsythe
Image Engineering Manager

Academy of Motion Picture Arts and Sciences
Science & Technology Council
1313 Vine Street • Hollywood, CA 90028

323.817.4170 • aforsythe@oscars.orgmailto:aforsythe@oscars.org

@aforsythe
Copy link
Member

@cbenhagen Still appears to be failing for me. First 3 tests pass if I use sudo, but last always fails.

https://gist.github.com/aforsythe/8ece55215b091d86c99d

Am I missing something?

On May 26, 2014, at 3:56 PM, Ben Hagen notifications@github.com wrote:

You can merge this Pull Request by running

git pull https://github.com/cbenhagen/CTL fix#45
Or view, comment on, or merge it at:

#46

Commit Summary

  • Use TARGET_FILE in ctlrender unittest. Fixes #45.

File Changes

  • M unittest/ctlrender/CMakeLists.txt (2)
  • M unittest/ctlrender/test.sh (7)

Patch Links:

  • https://github.com/ampas/CTL/pull/46.patch
  • https://github.com/ampas/CTL/pull/46.diff


Reply to this email directly or view it on GitHub.

Alex Forsythe
Image Engineering Manager
Academy of Motion Picture Arts and Sciences
1313 Vine Street Hollywood CA 90028
323.817.4170 mailto:aforsythe@oscars.org

@cbenhagen
Copy link
Contributor Author

Ummh that's strange. Can you show me the content of unittest/Testing/Temporary/LastTest.log?

@aforsythe
Copy link
Member

Looks like a path issue ...

Here's the result of running make check
https://gist.github.com/aforsythe/1e827a9b6f5f2ab2f2c0

And the result of running sudo make check
https://gist.github.com/aforsythe/8b0563ccf98e7dd641f4

@cbenhagen
Copy link
Contributor Author

so where is your ctlrender binary?

@aforsythe
Copy link
Member

I see the problem. It wasn't there because it doesn't get built when I run make check. I had to run make first. Still getting exceptions on tests 1 and 3 without sudo, but now 4 passes. All tests pass with sudo make check.

@cbenhagen
Copy link
Contributor Author

Cool so lets merge this and open another issue for the sudo problem. I can run all the tests without sudo here and i think you should never have to use sudo for tests.

@aforsythe
Copy link
Member

Agreed! Thanks for the help

@aforsythe aforsythe merged commit 729c157 into ampas:master May 27, 2014
@cbenhagen
Copy link
Contributor Author

You're welcome! Would you mind creating a 1.5.1 release with this fix? So i could finish the homebrew formula update?

@cbenhagen cbenhagen deleted the fix#45 branch May 27, 2014 15:30
@aforsythe
Copy link
Member

Done!!

@aforsythe
Copy link
Member

Will it mess anything up with the brew script if I merge #44 and move the 1.5.1 tag?

@cbenhagen
Copy link
Contributor Author

It would mess up the shasum of the release tarball but thats ok if you do it now 👍

@aforsythe
Copy link
Member

Give me just a couple of mins to update the documentation. I'll include new notes on how to install CTL via homebrew in the README.

Alex

On May 27, 2014, at 8:50 AM, Ben Hagen notifications@github.com wrote:

It would mess up the shasum of the release tarball but thats ok if you do it now


Reply to this email directly or view it on GitHub.

Alex Forsythe
Image Engineering Manager
Academy of Motion Picture Arts and Sciences
1313 Vine Street Hollywood CA 90028
323.817.4170 mailto:aforsythe@oscars.org

@aforsythe
Copy link
Member

Ok ...

I think we are good to go.

Alex

On May 27, 2014, at 8:50 AM, Ben Hagen notifications@github.com wrote:

It would mess up the shasum of the release tarball but thats ok if you do it now


Reply to this email directly or view it on GitHub.

Alex Forsythe
Image Engineering Manager
Academy of Motion Picture Arts and Sciences
1313 Vine Street Hollywood CA 90028
323.817.4170 mailto:aforsythe@oscars.org

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.

2 participants