Repository: oasis-open/cti-python-stix2
Pull request #154 (2018-04-02 20:47:45)

Increase code coverage, fix patterning bugs


Patch:
diff --git a/docs/conf.py b/docs/conf.py
index 49416a0..e53829b 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -74,7 +74,7 @@ def get_property_type(prop):
     return prop_class
 
 
-class STIXPropertyDocumenter(ClassDocumenter):
+class STIXAttributeDocumenter(ClassDocumenter):
     """Custom Sphinx extension to auto-document STIX properties.
 
     Needed because descendants of _STIXBase use `_properties` dictionaries
@@ -97,23 +97,16 @@ class STIXPropertyDocumenter(ClassDocumenter):
         obj = self.object
         self.add_line(':Properties:', '<stixattr>')
         for prop_name, prop in obj._properties.items():
-            # Skip 'type'
-            if prop_name == 'type':
-                continue
-
             # Add metadata about the property
             prop_type = get_property_type(prop)
             if prop_type == 'List':
                 prop_type = 'List of %ss' % get_property_type(prop.contained)
             if prop.required:
                 prop_type += ', required'
-            if 'Timestamp' in prop_type and hasattr(prop, 'default'):
-                prop_type += ', default: current date/time'
             prop_str = '**%s** (*%s*)' % (prop_name, prop_type)
             self.add_line('    - %s' % prop_str, '<stixattr>')
-
         self.add_line('', '<stixattr>')
 
 
 def setup(app):
-    app.add_autodocumenter(STIXPropertyDocumenter)
+    app.add_autodocumenter(STIXAttributeDocumenter)
diff --git a/setup.py b/setup.py
index 9700ec8..fa68616 100644
--- a/setup.py
+++ b/setup.py
@@ -4,12 +4,11 @@ import os.path
 
 from setuptools import find_packages, setup
 
-BASE_DIR = os.path.dirname(os.path.abspath(__file__))
-VERSION_FILE = os.path.join(BASE_DIR, 'stix2', 'version.py')
+here = os.path.abspath(os.path.dirname(__file__))
 
 
 def get_version():
-    with open(VERSION_FILE) as f:
+    with open('stix2/version.py', encoding="utf-8") as f:
         for line in f.readlines():
             if line.startswith("__version__"):
                 version = line.split()[-1].strip('"')
@@ -17,7 +16,7 @@ def get_version():
         raise AttributeError("Package does not have a __version__")
 
 
-with open('README.rst') as f:
+with open(os.path.join(here, 'README.rst'), encoding='utf-8') as f:
     long_description = f.read()
 
 
@@ -53,8 +52,6 @@ setup(
         'simplejson',
         'six',
         'stix2-patterns',
+        'taxii2-client',
     ],
-    extras_require={
-        'taxii': ['taxii2-client']
-    }
 )
diff --git a/stix2/__init__.py b/stix2/__init__.py
index 401d44b..9ab99d8 100644
--- a/stix2/__init__.py
+++ b/stix2/__init__.py
@@ -31,11 +31,12 @@ from .environment import Environment, ObjectFactory
 from .markings import (add_markings, clear_markings, get_markings, is_marked,
                        remove_markings, set_markings)
 from .patterns import (AndBooleanExpression, AndObservationExpression,
-                       BasicObjectPathComponent, EqualityComparisonExpression,
+                       BasicObjectPathComponent, BinaryConstant,
+                       BooleanConstant, EqualityComparisonExpression,
                        FloatConstant, FollowedByObservationExpression,
                        GreaterThanComparisonExpression,
                        GreaterThanEqualComparisonExpression, HashConstant,
-                       HexConstant, IntegerConstant,
+                       HexConstant, InComparisonExpression, IntegerConstant,
                        IsSubsetComparisonExpression,
                        IsSupersetComparisonExpression,
                        LessThanComparisonExpression,
diff --git a/stix2/base.py b/stix2/base.py
index 898f489..6bc3dd4 100644
--- a/stix2/base.py
+++ b/stix2/base.py
@@ -191,7 +191,7 @@ class _STIXBase(collections.Mapping):
             **kwargs: The arguments for a json.dumps() call.
 
         Returns:
-            str: The serialized JSON object.
+            dict: The serialized JSON object.
 
         Note:
             The argument ``pretty=True`` will output the STIX object following
diff --git a/stix2/datastore/filesystem.py b/stix2/datastore/filesystem.py
index a6f31cf..26d0c58 100644
--- a/stix2/datastore/filesystem.py
+++ b/stix2/datastore/filesystem.py
@@ -301,26 +301,11 @@ class FileSystemSource(DataSource):
         for path in include_paths:
             for root, dirs, files in os.walk(path):
                 for file_ in files:
-                    if not file_.endswith(".json"):
-                        # skip non '.json' files as more likely to be random non-STIX files
-                        continue
-
                     if not id_ or id_ == file_.split(".")[0]:
                         # have to load into memory regardless to evaluate other filters
-                        try:
-                            stix_obj = json.load(open(os.path.join(root, file_)))
-
-                            if stix_obj["type"] == "bundle":
-                                stix_obj = stix_obj["objects"][0]
-
-                            # naive STIX type checking
-                            stix_obj["type"]
-                            stix_obj["id"]
-
-                        except (ValueError, KeyError):  # likely not a JSON file
-                            raise TypeError("STIX JSON object at '{0}' could either not be parsed to "
-                                            "JSON or was not valid STIX JSON".format(os.path.join(root, file_)))
-
+                        stix_obj = json.load(open(os.path.join(root, file_)))
+                        if stix_obj.get('type', '') == 'bundle':
+                            stix_obj = stix_obj['objects'][0]
                         # check against other filters, add if match
                         all_data.extend(apply_common_filters([stix_obj], query))
 
diff --git a/stix2/datastore/filters.py b/stix2/datastore/filters.py
index 9065b61..5af48cd 100644
--- a/stix2/datastore/filters.py
+++ b/stix2/datastore/filters.py
@@ -35,11 +35,7 @@ def _check_filter_components(prop, op, value):
 
     if type(value) not in FILTER_VALUE_TYPES:
         # check filter value type is supported
-        raise TypeError("Filter value of '%s' is not supported. The type must be a Python immutable type or dictionary" % type(value))
-
-    if prop == "type" and "_" in value:
-        # check filter where the property is type, value (type name) cannot have underscores
-        raise ValueError("Filter for property 'type' cannot have its value '%s' include underscores" % value)
+        raise TypeError("Filter value type '%s' is not supported. The type must be a Python immutable type or dictionary" % type(value))
 
     return True
 
diff --git a/stix2/datastore/memory.py b/stix2/datastore/memory.py
index 1c4ff6c..e057271 100644
--- a/stix2/datastore/memory.py
+++ b/stix2/datastore/memory.py
@@ -295,5 +295,5 @@ class MemorySource(DataSource):
             for stix_obj in stix_data["objects"]:
                 _add(self, stix_data=parse(stix_obj, allow_custom=self.allow_custom, version=stix_data["spec_version"]))
         else:
-            _add(self, stix_data=parse(stix_data, allow_custom=self.allow_custom, version=version))
+            _add(self, stix_data=parse(stix_obj, allow_custom=self.allow_custom, version=version))
     load_from_file.__doc__ = MemoryStore.load_from_file.__doc__
diff --git a/stix2/datastore/taxii.py b/stix2/datastore/taxii.py
index 0a58763..5af4354 100644
--- a/stix2/datastore/taxii.py
+++ b/stix2/datastore/taxii.py
@@ -67,31 +67,27 @@ class TAXIICollectionSink(DataSink):
         """
         if isinstance(stix_data, _STIXBase):
             # adding python STIX object
-            if stix_data["type"] == "bundle":
-                bundle = stix_data.serialize(encoding="utf-8")
-            else:
-                bundle = Bundle(stix_data, allow_custom=self.allow_custom).serialize(encoding="utf-8")
+            bundle = dict(Bundle(stix_data, allow_custom=self.allow_custom))
 
         elif isinstance(stix_data, dict):
             # adding python dict (of either Bundle or STIX obj)
             if stix_data["type"] == "bundle":
-                bundle = parse(stix_data, allow_custom=self.allow_custom, version=version).serialize(encoding="utf-8")
+                bundle = stix_data
             else:
-                bundle = Bundle(stix_data, allow_custom=self.allow_custom).serialize(encoding="utf-8")
+                bundle = dict(Bundle(stix_data, allow_custom=self.allow_custom))
 
         elif isinstance(stix_data, list):
             # adding list of something - recurse on each
             for obj in stix_data:
                 self.add(obj, version=version)
-            return
 
         elif isinstance(stix_data, str):
             # adding json encoded string of STIX content
             stix_data = parse(stix_data, allow_custom=self.allow_custom, version=version)
             if stix_data["type"] == "bundle":
-                bundle = stix_data.serialize(encoding="utf-8")
+                bundle = dict(stix_data)
             else:
-                bundle = Bundle(stix_data, allow_custom=self.allow_custom).serialize(encoding="utf-8")
+                bundle = dict(Bundle(stix_data, allow_custom=self.allow_custom))
 
         else:
             raise TypeError("stix_data must be as STIX object(or list of),json formatted STIX (or list of), or a json formatted STIX bundle")
@@ -224,21 +220,18 @@ class TAXIICollectionSource(DataSource):
         if _composite_filters:
             query.update(_composite_filters)
 
-        # parse taxii query params (that can be applied remotely)
+        # separate taxii query terms (can be done remotely)
         taxii_filters = self._parse_taxii_filters(query)
 
-        # taxii2client requires query params as keywords
-        taxii_filters_dict = dict((f.property, f.value) for f in taxii_filters)
-
         # query TAXII collection
         try:
-            all_data = self.collection.get_objects(**taxii_filters_dict)["objects"]
+            all_data = self.collection.get_objects(filters=taxii_filters)["objects"]
 
             # deduplicate data (before filtering as reduces wasted filtering)
             all_data = deduplicate(all_data)
 
-            # apply local (CompositeDataSource, TAXIICollectionSource and query) filters
-            all_data = list(apply_common_filters(all_data, (query - taxii_filters)))
+            # apply local (CompositeDataSource, TAXIICollectionSource and query filters)
+            all_data = list(apply_common_filters(all_data, query))
 
         except HTTPError:
             # if resources not found or access is denied from TAXII server, return empty list
@@ -250,35 +243,30 @@ class TAXIICollectionSource(DataSource):
         return stix_objs
 
     def _parse_taxii_filters(self, query):
-        """Parse out TAXII filters that the TAXII server can filter on
-
-        Does not put in TAXII spec format as the TAXII2Client (that we use)
-        does this for us.
+        """Parse out TAXII filters that the TAXII server can filter on.
 
-        NOTE:
-            Currently, the TAXII2Client can handle TAXII filters where the
-            filter value is list, as both a comma-seperated string or python list
-
-            For instance - "?match[type]=indicator,sighting" can be in a
-            filter in any of these formats:
-
-            Filter("type", "<any op>", "indicator,sighting")
-
-            Filter("type", "<any op>", ["indicator", "sighting"])
+        Note:
+            For instance - "?match[type]=indicator,sighting" should be in a
+            query dict as follows:
 
+            Filter("type", "=", "indicator,sighting")
 
         Args:
-            query (set): set of filters to extract which ones are TAXII
+            query (list): list of filters to extract which ones are TAXII
                 specific.
 
         Returns:
-            taxii_filters (set): set of the TAXII filters
+            params (dict): dict of the TAXII filters but in format required
+                for 'requests.get()'.
 
         """
-        taxii_filters = set()
+        params = {}
 
         for filter_ in query:
             if filter_.property in TAXII_FILTERS:
-                taxii_filters.add(filter_)
-
-        return taxii_filters
+                if filter_.property == "added_after":
+                    params[filter_.property] = filter_.value
+                else:
+                    taxii_field = "match[%s]" % filter_.property
+                    params[taxii_field] = filter_.value
+        return params
diff --git a/stix2/environment.py b/stix2/environment.py
index eb5583e..dcf2449 100644
--- a/stix2/environment.py
+++ b/stix2/environment.py
@@ -114,16 +114,10 @@ class Environment(DataStoreMixin):
     create.__doc__ = ObjectFactory.create.__doc__
 
     def add_filters(self, *args, **kwargs):
-        try:
-            return self.source.filters.update(*args, **kwargs)
-        except AttributeError:
-            raise AttributeError('Environment has no data source')
+        return self.source.filters.update(*args, **kwargs)
 
     def add_filter(self, *args, **kwargs):
-        try:
-            return self.source.filters.add(*args, **kwargs)
-        except AttributeError:
-            raise AttributeError('Environment has no data source')
+        return self.source.filters.add(*args, **kwargs)
 
     def parse(self, *args, **kwargs):
         return _parse(*args, **kwargs)
diff --git a/stix2/patterns.py b/stix2/patterns.py
index 94ae7d2..23ce71b 100644
--- a/stix2/patterns.py
+++ b/stix2/patterns.py
@@ -3,8 +3,11 @@
 
 import base64
 import binascii
+import datetime
 import re
 
+from .utils import parse_into_datetime
+
 
 def escape_quotes_and_backslashes(s):
     return s.replace(u'\\', u'\\\\').replace(u"'", u"\\'")
@@ -24,10 +27,13 @@ class StringConstant(_Constant):
 
 class TimestampConstant(_Constant):
     def __init__(self, value):
-        self.value = value
+        try:
+            self.value = parse_into_datetime(value)
+        except Exception:
+            raise ValueError("must be a datetime object or timestamp string.")
 
     def __str__(self):
-        return "t'%s'" % escape_quotes_and_backslashes(self.value)
+        return "t%s" % repr(self.value)
 
 
 class IntegerConstant(_Constant):
@@ -46,7 +52,7 @@ class FloatConstant(_Constant):
         try:
             self.value = float(value)
         except Exception:
-            raise ValueError("must be an float.")
+            raise ValueError("must be a float.")
 
     def __str__(self):
         return "%s" % self.value
@@ -56,24 +62,29 @@ class BooleanConstant(_Constant):
     def __init__(self, value):
         if isinstance(value, bool):
             self.value = value
+            return
 
         trues = ['true', 't']
         falses = ['false', 'f']
         try:
             if value.lower() in trues:
                 self.value = True
-            if value.lower() in falses:
+                return
+            elif value.lower() in falses:
                 self.value = False
+                return
         except AttributeError:
             if value == 1:
                 self.value = True
-            if value == 0:
+                return
+            elif value == 0:
                 self.value = False
+                return
 
         raise ValueError("must be a boolean value.")
 
     def __str__(self):
-        return "%s" % self.value
+        return str(self.value).lower()
 
 
 _HASH_REGEX = {
@@ -132,20 +143,25 @@ class ListConstant(_Constant):
         self.value = values
 
     def __str__(self):
-        return "(" + ", ".join([("%s" % x) for x in self.value]) + ")"
+        return "(" + ", ".join([("%s" % make_constant(x)) for x in self.value]) + ")"
 
 
 def make_constant(value):
+    try:
+        return parse_into_datetime(value)
+    except ValueError:
+        pass
+
     if isinstance(value, str):
         return StringConstant(value)
+    elif isinstance(value, bool):
+        return BooleanConstant(value)
     elif isinstance(value, int):
         return IntegerConstant(value)
     elif isinstance(value, float):
         return FloatConstant(value)
     elif isinstance(value, list):
         return ListConstant(value)
-    elif isinstance(value, bool):
-        return BooleanConstant(value)
     else:
         raise ValueError("Unable to create a constant from %s" % value)
 
@@ -210,15 +226,12 @@ class ObjectPath(object):
 
 
 class _PatternExpression(object):
-
-    @staticmethod
-    def escape_quotes_and_backslashes(s):
-        return s.replace(u'\\', u'\\\\').replace(u"'", u"\\'")
+    pass
 
 
 class _ComparisonExpression(_PatternExpression):
     def __init__(self, operator, lhs, rhs, negated=False):
-        if operator == "=" and isinstance(rhs, ListConstant):
+        if operator == "=" and isinstance(rhs, (ListConstant, list)):
             self.operator = "IN"
         else:
             self.operator = operator
@@ -234,13 +247,6 @@ class _ComparisonExpression(_PatternExpression):
         self.root_type = self.lhs.object_type_name
 
     def __str__(self):
-        # if isinstance(self.rhs, list):
-        #     final_rhs = []
-        #     for r in self.rhs:
-        #         final_rhs.append("'" + self.escape_quotes_and_backslashes("%s" % r) + "'")
-        #     rhs_string = "(" + ", ".join(final_rhs) + ")"
-        # else:
-        #     rhs_string = self.rhs
         if self.negated:
             return "%s NOT %s %s" % (self.lhs, self.operator, self.rhs)
         else:
@@ -383,7 +389,7 @@ class RepeatQualifier(_ExpressionQualifier):
         elif isinstance(times_to_repeat, int):
             self.times_to_repeat = IntegerConstant(times_to_repeat)
         else:
-            raise ValueError("%s is not a valid argument for a Within Qualifier" % times_to_repeat)
+            raise ValueError("%s is not a valid argument for a Repeat Qualifier" % times_to_repeat)
 
     def __str__(self):
         return "REPEATS %s TIMES" % self.times_to_repeat
@@ -404,18 +410,18 @@ class WithinQualifier(_ExpressionQualifier):
 
 class StartStopQualifier(_ExpressionQualifier):
     def __init__(self, start_time, stop_time):
-        if isinstance(start_time, IntegerConstant):
+        if isinstance(start_time, TimestampConstant):
             self.start_time = start_time
-        elif isinstance(start_time, int):
-            self.start_time = IntegerConstant(start_time)
+        elif isinstance(start_time, datetime.date):
+            self.start_time = TimestampConstant(start_time)
         else:
-            raise ValueError("%s is not a valid argument for a Within Qualifier" % start_time)
-        if isinstance(stop_time, IntegerConstant):
+            raise ValueError("%s is not a valid argument for a Start/Stop Qualifier" % start_time)
+        if isinstance(stop_time, TimestampConstant):
             self.stop_time = stop_time
-        elif isinstance(stop_time, int):
-            self.stop_time = IntegerConstant(stop_time)
+        elif isinstance(stop_time, datetime.date):
+            self.stop_time = TimestampConstant(stop_time)
         else:
-            raise ValueError("%s is not a valid argument for a Within Qualifier" % stop_time)
+            raise ValueError("%s is not a valid argument for a Start/Stop Qualifier" % stop_time)
 
     def __str__(self):
         return "START %s STOP %s" % (self.start_time, self.stop_time)
diff --git a/tox.ini b/tox.ini
index ed26bc0..bfc8c1b 100644
--- a/tox.ini
+++ b/tox.ini
@@ -8,7 +8,6 @@ deps =
     pytest
     pytest-cov
     coverage
-    taxii2-client
 commands =
     py.test --cov=stix2 stix2/test/ --cov-report term-missing
 


Test patch:
diff --git a/stix2/test/test_datastore.py b/stix2/test/test_datastore.py
index e80e8d8..3490d6a 100644
--- a/stix2/test/test_datastore.py
+++ b/stix2/test/test_datastore.py
@@ -148,18 +148,18 @@ def test_parse_taxii_filters():
         Filter("created_by_ref", "=", "Bane"),
     ]
 
-    taxii_filters_expected = set([
-        Filter("added_after", "=", "2016-02-01T00:00:01.000Z"),
-        Filter("id", "=", "taxii stix object ID"),
-        Filter("type", "=", "taxii stix object ID"),
-        Filter("version", "=", "first")
-    ])
+    expected_params = {
+        "added_after": "2016-02-01T00:00:01.000Z",
+        "match[id]": "taxii stix object ID",
+        "match[type]": "taxii stix object ID",
+        "match[version]": "first"
+    }
 
     ds = taxii.TAXIICollectionSource(collection)
 
     taxii_filters = ds._parse_taxii_filters(query)
 
-    assert taxii_filters == taxii_filters_expected
+    assert taxii_filters == expected_params
 
 
 def test_add_get_remove_filter():
@@ -172,6 +172,22 @@ def test_add_get_remove_filter():
         Filter('labels', 'in', ["heartbleed", "malicious-activity"]),
     ]
 
+    # Invalid filters - wont pass creation
+    # these filters will not be allowed to be created
+    # check proper errors are raised when trying to create them
+
+    with pytest.raises(ValueError) as excinfo:
+        # create Filter that has an operator that is not allowed
+        Filter('modified', '*', 'not supported operator - just place holder')
+    assert str(excinfo.value) == "Filter operator '*' not supported for specified property: 'modified'"
+
+    with pytest.raises(TypeError) as excinfo:
+        # create Filter that has a value type that is not allowed
+        Filter('created', '=', object())
+    # On Python 2, the type of object() is `<type 'object'>` On Python 3, it's `<class 'object'>`.
+    assert str(excinfo.value).startswith("Filter value type")
+    assert str(excinfo.value).endswith("is not supported. The type must be a Python immutable type or dictionary")
+
     assert len(ds.filters) == 0
 
     ds.filters.add(valid_filters[0])
@@ -196,46 +212,6 @@ def test_add_get_remove_filter():
     ds.filters.update(valid_filters)
 
 
-def test_filter_ops_check():
-    # invalid filters - non supported operators
-
-    with pytest.raises(ValueError) as excinfo:
-        # create Filter that has an operator that is not allowed
-        Filter('modified', '*', 'not supported operator')
-    assert str(excinfo.value) == "Filter operator '*' not supported for specified property: 'modified'"
-
-    with pytest.raises(ValueError) as excinfo:
-        Filter("type", "%", "4")
-    assert "Filter operator '%' not supported for specified property" in str(excinfo.value)
-
-
-def test_filter_value_type_check():
-    # invalid filters - non supported value types
-
-    with pytest.raises(TypeError) as excinfo:
-        Filter('created', '=', object())
-    # On Python 2, the type of object() is `<type 'object'>` On Python 3, it's `<class 'object'>`.
-    assert any([s in str(excinfo.value) for s in ["<type 'object'>", "'<class 'object'>'"]])
-    assert "is not supported. The type must be a Python immutable type or dictionary" in str(excinfo.value)
-
-    with pytest.raises(TypeError) as excinfo:
-        Filter("type", "=", complex(2, -1))
-    assert any([s in str(excinfo.value) for s in ["<type 'complex'>", "'<class 'complex'>'"]])
-    assert "is not supported. The type must be a Python immutable type or dictionary" in str(excinfo.value)
-
-    with pytest.raises(TypeError) as excinfo:
-        Filter("type", "=", set([16, 23]))
-    assert any([s in str(excinfo.value) for s in ["<type 'set'>", "'<class 'set'>'"]])
-    assert "is not supported. The type must be a Python immutable type or dictionary" in str(excinfo.value)
-
-
-def test_filter_type_underscore_check():
-    # check that Filters where property="type", value (name) doesnt have underscores
-    with pytest.raises(ValueError) as excinfo:
-        Filter("type", "=", "oh_underscore")
-    assert "Filter for property 'type' cannot have its value 'oh_underscore'" in str(excinfo.value)
-
-
 def test_apply_common_filters():
     stix_objs = [
         {
diff --git a/stix2/test/test_filesystem.py b/stix2/test/test_filesystem.py
index f59136e..020fee5 100644
--- a/stix2/test/test_filesystem.py
+++ b/stix2/test/test_filesystem.py
@@ -1,4 +1,3 @@
-import json
 import os
 import shutil
 
@@ -46,41 +45,6 @@ def fs_sink():
     shutil.rmtree(os.path.join(FS_PATH, "campaign"), True)
 
 
-@pytest.fixture
-def bad_json_files():
-    # create erroneous JSON files for tests to make sure handled gracefully
-
-    with open(os.path.join(FS_PATH, "intrusion-set", "intrusion-set--test-non-json.txt"), "w+") as f:
-        f.write("Im not a JSON file")
-
-    with open(os.path.join(FS_PATH, "intrusion-set", "intrusion-set--test-bad-json.json"), "w+") as f:
-        f.write("Im not a JSON formatted file")
-
-    yield True  # dummy yield so can have teardown
-
-    os.remove(os.path.join(FS_PATH, "intrusion-set", "intrusion-set--test-non-json.txt"))
-    os.remove(os.path.join(FS_PATH, "intrusion-set", "intrusion-set--test-bad-json.json"))
-
-
-@pytest.fixture
-def bad_stix_files():
-    # create erroneous STIX JSON files for tests to make sure handled correctly
-
-    # bad STIX object
-    stix_obj = {
-        "id": "intrusion-set--test-bad-stix",
-        "spec_version": "2.0"
-        # no "type" field
-    }
-
-    with open(os.path.join(FS_PATH, "intrusion-set", "intrusion-set--test-non-stix.json"), "w+") as f:
-        f.write(json.dumps(stix_obj))
-
-    yield True  # dummy yield so can have teardown
-
-    os.remove(os.path.join(FS_PATH, "intrusion-set", "intrusion-set--test-non-stix.json"))
-
-
 @pytest.fixture(scope='module')
 def rel_fs_store():
     cam = Campaign(id=CAMPAIGN_ID, **CAMPAIGN_KWARGS)
@@ -112,26 +76,6 @@ def test_filesystem_sink_nonexistent_folder():
     assert "for STIX data does not exist" in str(excinfo)
 
 
-def test_filesystem_source_bad_json_file(fs_source, bad_json_files):
-    # this tests the handling of two bad json files
-    #  - one file should just be skipped (silently) as its a ".txt" extension
-    #  - one file should be parsed and raise Exception bc its not JSON
-    try:
-        fs_source.get("intrusion-set--test-bad-json")
-    except TypeError as e:
-        assert "intrusion-set--test-bad-json" in str(e)
-        assert "could either not be parsed to JSON or was not valid STIX JSON" in str(e)
-
-
-def test_filesystem_source_bad_stix_file(fs_source, bad_stix_files):
-    # this tests handling of bad STIX json object
-    try:
-        fs_source.get("intrusion-set--test-non-stix")
-    except TypeError as e:
-        assert "intrusion-set--test-non-stix" in str(e)
-        assert "could either not be parsed to JSON or was not valid STIX JSON" in str(e)
-
-
 def test_filesytem_source_get_object(fs_source):
     # get object
     mal = fs_source.get("malware--6b616fc1-1505-48e3-8b2c-0d19337bff38")
diff --git a/stix2/test/test_granular_markings.py b/stix2/test/test_granular_markings.py
index f8fc803..9e024a1 100644
--- a/stix2/test/test_granular_markings.py
+++ b/stix2/test/test_granular_markings.py
@@ -2,6 +2,7 @@
 import pytest
 
 from stix2 import TLP_RED, Malware, markings
+from stix2.exceptions import MarkingNotFoundError
 
 from .constants import MALWARE_MORE_KWARGS as MALWARE_KWARGS_CONST
 from .constants import MARKING_IDS
@@ -546,6 +547,20 @@ def test_remove_marking_bad_selector():
         markings.remove_markings(before, ["marking-definition--1", "marking-definition--2"], ["title"])
 
 
+def test_remove_marking_not_present():
+    before = Malware(
+        granular_markings=[
+            {
+                "selectors": ["description"],
+                "marking_ref": MARKING_IDS[0]
+            }
+        ],
+        **MALWARE_KWARGS
+    )
+    with pytest.raises(MarkingNotFoundError):
+        markings.remove_markings(before, [MARKING_IDS[1]], ["description"])
+
+
 IS_MARKED_TEST_DATA = [
     Malware(
         granular_markings=[
@@ -1044,3 +1059,10 @@ def test_clear_marking_bad_selector(data, selector):
     """Test bad selector raises exception."""
     with pytest.raises(AssertionError):
         markings.clear_markings(data, selector)
+
+
+@pytest.mark.parametrize("data", CLEAR_MARKINGS_TEST_DATA)
+def test_clear_marking_not_present(data):
+    """Test clearing markings for a selector that has no associated markings."""
+    with pytest.raises(MarkingNotFoundError):
+        data = markings.clear_markings(data, ["labels"])
diff --git a/stix2/test/test_pattern_expressions.py b/stix2/test/test_pattern_expressions.py
index 0db1083..363458a 100644
--- a/stix2/test/test_pattern_expressions.py
+++ b/stix2/test/test_pattern_expressions.py
@@ -1,3 +1,7 @@
+import datetime
+
+import pytest
+
 import stix2
 
 
@@ -67,7 +71,11 @@ def test_file_observable_expression():
     assert str(exp) == "[file:hashes.'SHA-256' = 'aec070645fe53ee3b3763059376134f058cc337247c978add178b6ccdfb0019f' AND file:mime_type = 'application/x-pdf']"  # noqa
 
 
-def test_multiple_file_observable_expression():
+@pytest.mark.parametrize("observation_class, op", [
+    (stix2.AndObservationExpression, 'AND'),
+    (stix2.OrObservationExpression, 'OR'),
+])
+def test_multiple_file_observable_expression(observation_class, op):
     exp1 = stix2.EqualityComparisonExpression("file:hashes.'SHA-256'",
                                               stix2.HashConstant(
                                                   "bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c",
@@ -81,8 +89,8 @@ def test_multiple_file_observable_expression():
                                                   'SHA-256'))
     op1_exp = stix2.ObservationExpression(bool1_exp)
     op2_exp = stix2.ObservationExpression(exp3)
-    exp = stix2.AndObservationExpression([op1_exp, op2_exp])
-    assert str(exp) == "[file:hashes.'SHA-256' = 'bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c' OR file:hashes.MD5 = 'cead3f77f6cda6ec00f57d76c9a6879f'] AND [file:hashes.'SHA-256' = 'aec070645fe53ee3b3763059376134f058cc337247c978add178b6ccdfb0019f']"  # noqa
+    exp = observation_class([op1_exp, op2_exp])
+    assert str(exp) == "[file:hashes.'SHA-256' = 'bf07a7fbb825fc0aae7bf4a1177b2b31fcf8a3feeaf7092761e18c859ee52a9c' OR file:hashes.MD5 = 'cead3f77f6cda6ec00f57d76c9a6879f'] {} [file:hashes.'SHA-256' = 'aec070645fe53ee3b3763059376134f058cc337247c978add178b6ccdfb0019f']".format(op)  # noqa
 
 
 def test_root_types():
@@ -120,6 +128,31 @@ def test_greater_than():
     assert str(exp) == "[file:extensions.windows-pebinary-ext.sections[*].entropy > 7.0]"
 
 
+def test_less_than():
+    exp = stix2.LessThanComparisonExpression("file:size",
+                                             1024)
+    assert str(exp) == "file:size < 1024"
+
+
+def test_greater_than_or_equal():
+    exp = stix2.GreaterThanEqualComparisonExpression("file:size",
+                                                     1024)
+    assert str(exp) == "file:size >= 1024"
+
+
+def test_less_than_or_equal():
+    exp = stix2.LessThanEqualComparisonExpression("file:size",
+                                                  1024)
+    assert str(exp) == "file:size <= 1024"
+
+
+def test_not():
+    exp = stix2.LessThanComparisonExpression("file:size",
+                                             1024,
+                                             negated=True)
+    assert str(exp) == "file:size NOT < 1024"
+
+
 def test_and_observable_expression():
     exp1 = stix2.AndBooleanExpression([stix2.EqualityComparisonExpression("user-account:account_type",
                                                                           "unix"),
@@ -145,6 +178,15 @@ def test_and_observable_expression():
     assert str(exp) == "[user-account:account_type = 'unix' AND user-account:user_id = '1007' AND user-account:account_login = 'Peter'] AND [user-account:account_type = 'unix' AND user-account:user_id = '1008' AND user-account:account_login = 'Paul'] AND [user-account:account_type = 'unix' AND user-account:user_id = '1009' AND user-account:account_login = 'Mary']"  # noqa
 
 
+def test_invalid_and_observable_expression():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.AndBooleanExpression([stix2.EqualityComparisonExpression("user-account:display_name",
+                                                                       "admin"),
+                                    stix2.EqualityComparisonExpression("email-addr:display_name",
+                                                                       stix2.StringConstant("admin"))])
+    assert "All operands to an 'AND' expression must have the same object type" in str(excinfo)
+
+
 def test_hex():
     exp_and = stix2.AndBooleanExpression([stix2.EqualityComparisonExpression("file:mime_type",
                                                                              "image/bmp"),
@@ -175,3 +217,158 @@ def test_set_op():
 def test_timestamp():
     ts = stix2.TimestampConstant('2014-01-13T07:03:17Z')
     assert str(ts) == "t'2014-01-13T07:03:17Z'"
+
+
+def test_boolean():
+    exp = stix2.EqualityComparisonExpression("email-message:is_multipart",
+                                             True)
+    assert str(exp) == "email-message:is_multipart = true"
+
+
+def test_binary():
+    const = stix2.BinaryConstant("dGhpcyBpcyBhIHRlc3Q=")
+    exp = stix2.EqualityComparisonExpression("artifact:payload_bin",
+                                             const)
+    assert str(exp) == "artifact:payload_bin = b'dGhpcyBpcyBhIHRlc3Q='"
+
+
+def test_list():
+    exp = stix2.InComparisonExpression("process:name",
+                                       ['proccy', 'proximus', 'badproc'])
+    assert str(exp) == "process:name IN ('proccy', 'proximus', 'badproc')"
+
+
+def test_list2():
+    # alternate way to construct an "IN" Comparison Expression
+    exp = stix2.EqualityComparisonExpression("process:name",
+                                             ['proccy', 'proximus', 'badproc'])
+    assert str(exp) == "process:name IN ('proccy', 'proximus', 'badproc')"
+
+
+def test_invalid_constant_type():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.EqualityComparisonExpression("artifact:payload_bin",
+                                           {'foo': 'bar'})
+    assert 'Unable to create a constant' in str(excinfo)
+
+
+def test_invalid_integer_constant():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.IntegerConstant('foo')
+    assert 'must be an integer' in str(excinfo)
+
+
+def test_invalid_timestamp_constant():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.TimestampConstant('foo')
+    assert 'must be a datetime object or timestamp string' in str(excinfo)
+
+
+def test_invalid_float_constant():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.FloatConstant('foo')
+    assert 'must be a float' in str(excinfo)
+
+
+@pytest.mark.parametrize("data, result", [
+    (True, True),
+    (False, False),
+    ('True', True),
+    ('False', False),
+    ('true', True),
+    ('false', False),
+    ('t', True),
+    ('f', False),
+    ('T', True),
+    ('F', False),
+    (1, True),
+    (0, False),
+])
+def test_boolean_constant(data, result):
+    boolean = stix2.BooleanConstant(data)
+    assert boolean.value == result
+
+
+def test_invalid_boolean_constant():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.BooleanConstant('foo')
+    assert 'must be a boolean' in str(excinfo)
+
+
+@pytest.mark.parametrize("hashtype, data", [
+    ('MD5', 'zzz'),
+    ('ssdeep', 'zzz=='),
+])
+def test_invalid_hash_constant(hashtype, data):
+    with pytest.raises(ValueError) as excinfo:
+        stix2.HashConstant(data, hashtype)
+    assert 'is not a valid {} hash'.format(hashtype) in str(excinfo)
+
+
+def test_invalid_hex_constant():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.HexConstant('mm')
+    assert "must contain an even number of hexadecimal characters" in str(excinfo)
+
+
+def test_invalid_binary_constant():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.BinaryConstant('foo')
+    assert 'must contain a base64' in str(excinfo)
+
+
+def test_escape_quotes_and_backslashes():
+    exp = stix2.MatchesComparisonExpression("file:name",
+                                            "^Final Report.+\.exe$")
+    assert str(exp) == "file:name MATCHES '^Final Report.+\\\\.exe$'"
+
+
+def test_like():
+    exp = stix2.LikeComparisonExpression("directory:path",
+                                         "C:\Windows\%\\foo")
+    assert str(exp) == "directory:path LIKE 'C:\\\\Windows\\\\%\\\\foo'"
+
+
+def test_issuperset():
+    exp = stix2.IsSupersetComparisonExpression("ipv4-addr:value",
+                                               "198.51.100.0/24")
+    assert str(exp) == "ipv4-addr:value ISSUPERSET '198.51.100.0/24'"
+
+
+def test_repeat_qualifier():
+    qual = stix2.RepeatQualifier(stix2.IntegerConstant(5))
+    assert str(qual) == 'REPEATS 5 TIMES'
+
+
+def test_invalid_repeat_qualifier():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.RepeatQualifier('foo')
+    assert 'is not a valid argument for a Repeat Qualifier' in str(excinfo)
+
+
+def test_invalid_within_qualifier():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.WithinQualifier('foo')
+    assert 'is not a valid argument for a Within Qualifier' in str(excinfo)
+
+
+def test_startstop_qualifier():
+    qual = stix2.StartStopQualifier(stix2.TimestampConstant('2016-06-01T00:00:00Z'),
+                                    datetime.datetime(2017, 3, 12, 8, 30, 0))
+    assert str(qual) == "START t'2016-06-01T00:00:00Z' STOP t'2017-03-12T08:30:00Z'"
+
+    qual2 = stix2.StartStopQualifier(datetime.date(2016, 6, 1),
+                                     stix2.TimestampConstant('2016-07-01T00:00:00Z'))
+    assert str(qual2) == "START t'2016-06-01T00:00:00Z' STOP t'2016-07-01T00:00:00Z'"
+
+
+def test_invalid_startstop_qualifier():
+    with pytest.raises(ValueError) as excinfo:
+        stix2.StartStopQualifier('foo',
+                                 stix2.TimestampConstant('2016-06-01T00:00:00Z'))
+    assert 'is not a valid argument for a Start/Stop Qualifier' in str(excinfo)
+
+    with pytest.raises(ValueError) as excinfo:
+        stix2.StartStopQualifier(datetime.date(2016, 6, 1),
+                                 'foo')
+    assert 'is not a valid argument for a Start/Stop Qualifier' in str(excinfo)
diff --git a/stix2/test/test_properties.py b/stix2/test/test_properties.py
index 6bd1888..34edc96 100644
--- a/stix2/test/test_properties.py
+++ b/stix2/test/test_properties.py
@@ -16,6 +16,8 @@ def test_property():
     p = Property()
 
     assert p.required is False
+    assert p.clean('foo') == 'foo'
+    assert p.clean(3) == 3
 
 
 def test_basic_clean():
diff --git a/stix2/test/test_versioning.py b/stix2/test/test_versioning.py
index 233587e..fa3bddb 100644
--- a/stix2/test/test_versioning.py
+++ b/stix2/test/test_versioning.py
@@ -233,12 +233,19 @@ def test_remove_custom_stix_object():
         ("animal_class", stix2.properties.StringProperty()),
     ])
     class Animal(object):
-        def __init__(self, animal_class=None, **kwargs):
-            if animal_class and animal_class not in ["mammal", "bird"]:
-                raise ValueError("Not a recognized class of animal")
+        pass
 
     animal = Animal(species="lion", animal_class="mammal")
 
     nc = stix2.utils.remove_custom_stix(animal)
 
     assert nc is None
+
+
+def test_remove_custom_stix_no_custom():
+    campaign_v1 = stix2.Campaign(**CAMPAIGN_MORE_KWARGS)
+    campaign_v2 = stix2.utils.remove_custom_stix(campaign_v1)
+
+    assert len(campaign_v1.keys()) == len(campaign_v2.keys())
+    assert campaign_v1.id == campaign_v2.id
+    assert campaign_v1.description == campaign_v2.description