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.

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

Jump to bottom

Add snapcraft-parser integration test. #560

Merged
merged 4 commits into from
Jun 14, 2016
Merged

Add snapcraft-parser integration test. #560

merged 4 commits into from
Jun 14, 2016

Conversation

josepht
Copy link
Contributor

@josepht josepht commented Jun 9, 2016

Add a "Happy Path" integration test for snapcraft-parser.
LP: #1590268

@snappy-m-o
Copy link

Can one of the admins verify this patch?

1 similar comment
@snappy-m-o
Copy link

Can one of the admins verify this patch?

come-maiz
come-maiz reviewed Jun 10, 2016
View reviewed changes
integration_tests/test_parser.py


class TestParser(TestCase):
'''Test bin/snapcraft-parser'''
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, the comments are with three double quotes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@come-maiz
Copy link
Contributor

Great, thanks.
I think I would like to get this to check parts of the output, but I don't like the current output. And I think I would like to check parts of the resulting file. Both things would fail if the index changes, so maybe it would be better to add a test later with a local index, and leave the one accessing the wiki simple. 👍 after the comments are fixed.

come-maiz
come-maiz reviewed Jun 10, 2016
View reviewed changes
integration_tests/test_parser.py
'https://wiki.ubuntu.com/snapcraft/parts?action=raw',
'--output', 'parts.yaml']
with open(os.path.devnull, 'w') as devnull:
subprocess.check_call(args, stderr=devnull, stdout=devnull)
Copy link
Contributor

Choose a reason for hiding this comment

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

use subprocess.DEVNULL

@josepht
Copy link
Contributor Author

josepht commented Jun 10, 2016

@ElOpio I've updated based on your comments. Thanks for the review.

@come-maiz
Copy link
Contributor

👍

@josepht
Add snapcraft-parser integration test.
0b39b39 
Add a "Happy Path" integration test for snapcraft-parser.
LP: #1590268
@josepht
Cleanup based on reviews.
2988f11 
* Inherit from integration_tests.TestCase
* Use subprocess.DEVNULL rather than opening /dev/null explicitly
* Use double quotes for test docstring comments.
* Add snapcraft_parser_command to integration_test.TestCase so
  autopkgtests can use the installed version.
@snappy-m-o
Copy link

Can one of the admins verify this patch?

1 similar comment
@snappy-m-o
Copy link

Can one of the admins verify this patch?

@sergiusens
Copy link
Collaborator

OK to test

@sergiusens sergiusens merged commit 8f5b30d into canonical:master Jun 14, 2016
@josepht josepht deleted the bug_1590268 branch June 23, 2016 14:23
kalikiana pushed a commit to kalikiana/snapcraft that referenced this pull request Apr 6, 2017
@josepht @sergiusens
Add snapcraft-parser integration test. (canonical#560)
d3dd1ab 
Add a "Happy Path" integration test for snapcraft-parser.

LP: #1590268
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@josepht @snappy-m-o @come-maiz @sergiusens