Skip to content

Fix build failure for Travis, Appveyor and Circle #36 #37

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix build failure for Travis, Appveyor and Circle #36 #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fecae3
Fix build failure for Travis and Appveyor #36
yash-nisar Mar 4, 2018
b13408a
Fix build failure for CircleCI #36
yash-nisar Mar 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions circle.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
machine:
pre:
- brew update
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please explain in the commit message why this is necessary, because AFAIK brew install does an auto-update.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some useful threads which I found were -> https://discuss.circleci.com/t/homebrew-must-be-run-under-ruby-2-3-runtimeerror/17232 and Homebrew/brew#3299 (comment)

- python --version
- sudo -H pip install --upgrade virtualenv
- brew install pyenv
Expand Down
4 changes: 2 additions & 2 deletions tests/test_document.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def test_write_document_tv_with_validate(self):
try:
temp_dir = tempfile.mkdtemp(prefix='test_spdx')
result_file = os.path.join(temp_dir, 'spdx-simple.tv')
with open(result_file, 'wb') as output:
with open(result_file, 'w') as output:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not a Python expert: Will this work for Python 2, too? Because we need to stay compatible to both versions 2 and 3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will work on Python 2 too. The tests pass for Python 2.7. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I'm wondering why it ever worked for Python 3. Do you have any idea about that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, Python 3 returns 'bytes' and not 'str' when reading/writing to/from a binary stream. So, the files need to be opened in text mode for str related operations.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sschuberth I guess the answer is in the Travis Builds - If you look at older builds that passed the tests hadn't been added up till then (back when the builds were passing that is)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't realize @pombredanne had committed this directly, without going through a PR that runs CI. Otherwise that change should have never gotten in.

write_document(doc, output, validate=True)

expected_file = utils_test.get_test_loc(
Expand All @@ -211,7 +211,7 @@ def test_write_document_tv_with_or_later_with_validate(self):
result_file = os.path.join(temp_dir, 'spdx-simple-plus.tv')

# test proper!
with open(result_file, 'wb') as output:
with open(result_file, 'w') as output:
write_document(doc, output, validate=True)

expected_file = utils_test.get_test_loc(
Expand Down