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

tests with mocking #20

Merged
merged 3 commits into from
May 2, 2014
Merged

tests with mocking #20

merged 3 commits into from
May 2, 2014

Conversation

cfaessler
Copy link
Contributor

@dbrgn can you please review as discussed. thx.

from swid_generator.environments.rpm_environment import RPMEnvironment
from swid_generator.environments.common import CommonEnvironment
from swid_generator.generators.swid_generator import create_swid_tags
from xml.etree import cElementTree as ET
Copy link
Collaborator

Choose a reason for hiding this comment

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

cElementTree is not guaranteed to be installed. Use this instead:

try:
    from xml.etree import cElementTree as ET
except ImportError:
    from xml.etree import ElementTree as ET

@dbrgn
Copy link
Collaborator

dbrgn commented Apr 28, 2014

I'd recommend converting test_SwidGenerator and test_SwidGeneratorParser to under_scored variants for consistency reasons.

What exactly is the difference between samples and dumps? Also, I find the naming for all the text files very confusing. I could never tell what the content is just by looking at the name. Maybe subfolders (like softwareids, directory_listings, swid_tags etc) would help?

I'd try to organize the rpm_fixtures.py file a bit better, by grouping them and adding comments (I always use ### Foo Bar ### on a single line to divide functions of different groups). Examples could be Helper Functions, Environment Fixtures or XML Fixtures.

Also, maybe it would make sense to create a fixtures package and move rpm_fixtures.py to fixtures/rpm_environment.py. (Don't forget the __init__.py file.)

Btw, minimock rocks! :)

@cfaessler
Copy link
Contributor Author

@dbrgn implemented the suggested changes

@dbrgn
Copy link
Collaborator

dbrgn commented May 1, 2014

Besides that issue above, I think this is ready to rebase & merge! :)

Dump file organization is much better now.

@dbrgn dbrgn assigned cfaessler and unassigned dbrgn May 1, 2014
- dumps of rpm and dpkg output
- dumps of swid and software-id output
@cfaessler cfaessler merged commit 5cd9372 into master May 2, 2014
@cfaessler cfaessler deleted the test-mock branch May 2, 2014 09:41
@cfaessler cfaessler removed their assignment Jun 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants