Checking for subdomains, fixing more ip logic stuff and tests

src/registrar/forms/domain.py

@@ -21,7 +21,8 @@ class DomainNameserverForm(forms.Form):
     """Form for changing nameservers."""
 
     server = forms.CharField(label="Name server", strip=True)
-    #when adding IPs to this form ensure they are stripped as well
+    # when adding IPs to this form ensure they are stripped as well
+
 
 NameserverFormset = formset_factory(
     DomainNameserverForm,

src/registrar/models/domain.py

@@ -228,8 +228,6 @@ class Domain(TimeStampedModel, DomainHelper):
         """
         try:
             hosts = self._get_property("hosts")
-            print("HOST IS ")
-            print(hosts)
         except Exception as err:
             # TODO-848: Check/add to error handling ticket if it's not addressed
             # (Don't throw error as this is normal for a new domain?)
@@ -238,7 +236,7 @@ class Domain(TimeStampedModel, DomainHelper):
 
         hostList = []
         for host in hosts:
-            hostList.append((host["name"],host["addrs"]))
+            hostList.append((host["name"], host["addrs"]))
 
         return hostList
 
@@ -267,14 +265,11 @@ class Domain(TimeStampedModel, DomainHelper):
         returns ErrorCode (int)"""
         logger.info("Creating host")
         if addrs is not None:
-            # TODO-848: Make sure to have 1 with ip address + 1 without
             addresses = [epp.Ip(addr=addr) for addr in addrs]
             request = commands.CreateHost(name=host, addrs=addresses)
         else:
-            # NOTE-848: ip is a specification within the nameserver
             request = commands.CreateHost(name=host)
 
-        # NOTE-848: if you talk to registry you MUST do try/except
         try:
             logger.info("_create_host()-> sending req as %s" % request)
             response = registry.send(request, cleaned=True)
@@ -284,7 +279,9 @@ class Domain(TimeStampedModel, DomainHelper):
             return e.code
 
     def _convert_list_to_dict(self, listToConvert: list[tuple[str]]):
-        newDict={}
+        newDict = {}
+
+        # TODO-848: If duplicated nameserver names, throw error
         for tup in listToConvert:
             if len(tup) == 1:
                 newDict[tup[0]] = None
@@ -292,16 +289,20 @@ class Domain(TimeStampedModel, DomainHelper):
                 newDict[tup[0]] = tup[1]
         return newDict
 
-    def isDotGov(self, nameserver):
+    def isSubdomain(self, nameserver):
-        return nameserver.find(".gov")!=-1
+        return nameserver.find(self.name) != -1
 
-    def checkHostIPCombo(self, nameserver:str, ip:list):
+    def checkHostIPCombo(self, nameserver: str, ip: list):
-        if ( self.isDotGov(nameserver) and (ip is None or
+        if self.isSubdomain(nameserver) and (ip is None or ip == []):
-            ip==[]) ):
+            raise ValueError("Nameserver %s needs to have an ip address" % nameserver)
-            raise ValueError("Nameserver %s needs to have an ip address", nameserver)
+        elif not self.isSubdomain(nameserver) and (ip is not None and ip != []):
+            raise ValueError(
+                "Nameserver %s cannot be linked "
+                "because %s is not a subdomain" % (nameserver, ip)
+            )
         return None
 
-    def getNameserverChanges(self, hosts:list[tuple[str]]):
+    def getNameserverChanges(self, hosts: list[tuple[str]]):
         """
         calls self.nameserver, it should pull from cache but may result
         in an epp call
@@ -310,7 +311,7 @@ class Domain(TimeStampedModel, DomainHelper):
         updated_values:
         new_values: dict
         oldNameservers:"""
-        oldNameservers=self.nameservers
+        oldNameservers = self.nameservers
 
         previousHostDict = self._convert_list_to_dict(oldNameservers)
 
@@ -320,31 +321,31 @@ class Domain(TimeStampedModel, DomainHelper):
         new_values = []
 
         for prevHost in previousHostDict:
-            addrs=previousHostDict[prevHost]
+            addrs = previousHostDict[prevHost]
             # get deleted values-which are values in previous nameserver list
             # but are not in the list of new host values
             if prevHost not in newHostDict:
-                deleted_values.append((prevHost,addrs))
+                deleted_values.append((prevHost, addrs))
             # if the host exists in both, check if the addresses changed
             else:
-                #TODO - host is being updated when previous was None and new is an empty list
+                # TODO - host is being updated when previous was None and new is an empty list
                 # add check here
-                if (newHostDict[prevHost] != addrs 
+                if newHostDict[prevHost] is not None and set(
-                    and newHostDict[prevHost] is not None): 
+                    newHostDict[prevHost]
-                    # could raise error here if new value is empty and is a dotgov
+                ) != set(addrs):
                     self.checkHostIPCombo(nameserver=prevHost, ip=newHostDict[prevHost])
-                    updated_values.append((prevHost,newHostDict[prevHost]))
+                    updated_values.append((prevHost, newHostDict[prevHost]))
 
-        new_values=set(newHostDict)-set(previousHostDict) #returns actually a set
+        new_values = {
+            key: newHostDict.get(key)
+            for key in newHostDict
+            if key not in previousHostDict
+        }
 
-        final_new_values = dict.fromkeys(new_values, None)
+        for nameserver, ip in new_values.items():
-        # loop in final new values to check for .gov and missing addresses
+            self.checkHostIPCombo(nameserver=nameserver, ip=ip)
-        for nameserver, ip in final_new_values.items():
-            # check the new values for missing IPs
-            # raise error if missing
-            self.checkHostIPCombo(nameserver=nameserver,ip=ip)
 
-        return (deleted_values,updated_values,final_new_values, previousHostDict)
+        return (deleted_values, updated_values, new_values, previousHostDict)
 
     @nameservers.setter  # type: ignore
     def nameservers(self, hosts: list[tuple[str]]):
@@ -354,11 +355,6 @@ class Domain(TimeStampedModel, DomainHelper):
 
         # TODO-848: ip version checking may need to be added in a different ticket
 
-        # We currently don't have IP address functionality
-        # We can have multiple IP addresses 
-        # If you have a dotgov, then you HAVE to have at least 1 IP address (can have multiple)
-        # Nameservers already have IP addresses, these IP addresses are additionals
-
         if len(hosts) > 13:
             raise ValueError(
                 "Too many hosts provided, you may not have more than 13 nameservers."
@@ -366,8 +362,13 @@ class Domain(TimeStampedModel, DomainHelper):
         logger.info("Setting nameservers")
         logger.info(hosts)
 
-        #get the changes made by user and old nameserver values
+        # get the changes made by user and old nameserver values
-        deleted_values, updated_values, new_values, oldNameservers=self.getNameserverChanges(hosts=hosts)
+        (
+            deleted_values,
+            updated_values,
+            new_values,
+            oldNameservers,
+        ) = self.getNameserverChanges(hosts=hosts)
         successDeletedCount = 0
         successCreatedCount = 0
 
@@ -380,29 +381,31 @@ class Domain(TimeStampedModel, DomainHelper):
         print("oldNameservers")
         print(oldNameservers)
 
-
         for hostTuple in deleted_values:
-            print("hostTuple in deleted_values")
-            print(hostTuple)
-            print(deleted_values)
             deleted_response_code = self._delete_host(hostTuple[0])
             if deleted_response_code == ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY:
                 successDeletedCount += 1
 
         for hostTuple in updated_values:
-            updated_response_code = self._update_host(hostTuple[0], hostTuple[1], oldNameservers.get(hostTuple[0]))
+            updated_response_code = self._update_host(
-            if updated_response_code not in [ ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY, ErrorCode.OBJECT_EXISTS]:
+                hostTuple[0], hostTuple[1], oldNameservers.get(hostTuple[0])
-                logger.warning("Could not update host %s. Error code was: %s " % (hostTuple[0], updated_response_code))
+            )
+            if updated_response_code not in [
+                ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY,
+                ErrorCode.OBJECT_EXISTS,
+            ]:
+                logger.warning(
+                    "Could not update host %s. Error code was: %s "
+                    % (hostTuple[0], updated_response_code)
+                )
         for key, value in new_values.items():
-            print("HELLO THERE KEY, VALUE PAIR")
+            createdCode = self._create_host(
-            print(key)
+                host=key, addrs=value
-            print(value)
+            )  # creates in registry
-            createdCode = self._create_host(host=key, addrs=value) # creates in registry
+            if (
-            # TODO-848: Double check if _create_host should handle duplicates + update domain obj?
+                createdCode == ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY
-            # NOTE-848: if createdCode == ErrorCode.OBJECT_EXISTS: --> self.nameservers
+                or createdCode == ErrorCode.OBJECT_EXISTS
-
+            ):
-            # NOTE-848: Host can be used by multiple domains
-            if createdCode == ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY or createdCode == ErrorCode.OBJECT_EXISTS:
                 request = commands.UpdateDomain(
                     name=self.name, add=[epp.HostObjSet([key])]
                 )
@@ -415,8 +418,9 @@ class Domain(TimeStampedModel, DomainHelper):
                         % (e.code, e)
                     )
 
-        print("")
+        successTotalNameservers = (
-        successTotalNameservers = len(oldNameservers) - successDeletedCount+ successCreatedCount
+            len(oldNameservers) - successDeletedCount + successCreatedCount
+        )
 
         print("len(oldNameservers) IS ")
         print(len(oldNameservers))
@@ -425,9 +429,6 @@ class Domain(TimeStampedModel, DomainHelper):
         print("successCreatedCount IS ")
         print(successCreatedCount)
 
-
-
-
         print("SUCCESSTOTALNAMESERVERS IS ")
         print(successTotalNameservers)
         if successTotalNameservers < 2:
@@ -438,17 +439,19 @@ class Domain(TimeStampedModel, DomainHelper):
             except Exception as err:
                 logger.info(
                     "nameserver setter checked for dns_needed state "
-                    "and it did not succeed. Error: %s" % err
+                    "and it did not succeed. Warning: %s" % err
                 )
         elif successTotalNameservers >= 2 and successTotalNameservers <= 13:
             try:
-                print("READY/SAVE: We are in happy path where btwen 2 and 13 inclusive ns")
+                print(
+                    "READY/SAVE: We are in happy path where btwen 2 and 13 inclusive ns"
+                )
                 self.ready()
                 self.save()
             except Exception as err:
                 logger.info(
                     "nameserver setter checked for create state "
-                    "and it did not succeed. Error: %s" % err
+                    "and it did not succeed. Warning: %s" % err
                 )
         # TODO-848: Handle removed nameservers here, will need to change the state then go back to DNS_NEEDED
 
@@ -1089,7 +1092,7 @@ class Domain(TimeStampedModel, DomainHelper):
             return []
 
         for ip_addr in ip_list:
-            if self.is_ipv6():
+            if self.is_ipv6(ip_addr):
                 edited_ip_list.append(epp.Ip(addr=ip_addr, ip="v6"))
             else:  # default ip addr is v4
                 edited_ip_list.append(epp.Ip(addr=ip_addr))
@@ -1098,11 +1101,22 @@ class Domain(TimeStampedModel, DomainHelper):
 
     def _update_host(self, nameserver: str, ip_list: list[str], old_ip_list: list[str]):
         try:
-
+            if (
-            if ip_list is None or len(ip_list) == 0 and isinstance(old_ip_list,list) and len(old_ip_list)!=0 :
+                ip_list is None
+                or len(ip_list) == 0
+                and isinstance(old_ip_list, list)
+                and len(old_ip_list) != 0
+            ):
                 return ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY
 
-            request = commands.UpdateHost(name=nameserver, add=self._convert_ips(ip_list), rem=self._convert_ips(old_ip_list))
+            added_ip_list = set(ip_list) - set(old_ip_list)
+            removed_ip_list = set(old_ip_list) - set(ip_list)
+
+            request = commands.UpdateHost(
+                name=nameserver,
+                add=self._convert_ips(list(added_ip_list)),
+                rem=self._convert_ips(list(removed_ip_list)),
+            )
             response = registry.send(request, cleaned=True)
             logger.info("_update_host()-> sending req as %s" % request)
             return response.code
@@ -1115,19 +1129,26 @@ class Domain(TimeStampedModel, DomainHelper):
             updateReq = commands.UpdateDomain(
                 name=self.name, rem=[epp.HostObjSet([nameserver])]
             )
-            response=registry.send(updateReq, cleaned=True)
+            response = registry.send(updateReq, cleaned=True)
 
             logger.info("_delete_host()-> sending update domain req as %s" % updateReq)
 
             deleteHostReq = commands.DeleteHost(name=nameserver)
             response = registry.send(deleteHostReq, cleaned=True)
-            logger.info("_delete_host()-> sending delete host req as %s" % deleteHostReq)
+            logger.info(
+                "_delete_host()-> sending delete host req as %s" % deleteHostReq
+            )
             return response.code
         except RegistryError as e:
-            if e.code==ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION:
+            if e.code == ErrorCode.OBJECT_ASSOCIATION_PROHIBITS_OPERATION:
-                logger.info("Did not remove host %s because it is in use on another domain." % nameserver)
+                logger.info(
+                    "Did not remove host %s because it is in use on another domain."
+                    % nameserver
+                )
             else:
-                logger.error("Error _delete_host, code was %s error was %s" % (e.code, e))
+                logger.error(
+                    "Error _delete_host, code was %s error was %s" % (e.code, e)
+                )
                 return e.code
 
     def _fetch_cache(self, fetch_hosts=False, fetch_contacts=False):

src/registrar/tests/common.py

@@ -562,7 +562,7 @@ class MockEppLib(TestCase):
             self.contacts = contacts
             self.hosts = hosts
             self.statuses = statuses
-            self.avail = avail #use for CheckDomain
+            self.avail = avail  # use for CheckDomain
             self.addrs = addrs
 
     mockDataInfoDomain = fakedEppObject(
@@ -581,36 +581,52 @@ class MockEppLib(TestCase):
         contacts=[],
         hosts=["fake.host.com"],
     )
-    infoDomainThreeHosts =fakedEppObject(
+    infoDomainThreeHosts = fakedEppObject(
         "my-nameserver.gov",
         cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
         contacts=[],
-        hosts=["ns1.my-nameserver-1.com","ns1.my-nameserver-2.com","ns1.cats-are-superior3.com"],
+        hosts=[
+            "ns1.my-nameserver-1.com",
+            "ns1.my-nameserver-2.com",
+            "ns1.cats-are-superior3.com",
+        ],
     )
-    infoDomainNoHost =fakedEppObject(
+    infoDomainNoHost = fakedEppObject(
         "my-nameserver.gov",
         cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
         contacts=[],
         hosts=[],
     )
-    infoDomainTwoHosts =fakedEppObject(
+    infoDomainTwoHosts = fakedEppObject(
         "my-nameserver.gov",
         cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
         contacts=[],
-        hosts=["ns1.my-nameserver-1.com","ns1.my-nameserver-2.com"],
+        hosts=["ns1.my-nameserver-1.com", "ns1.my-nameserver-2.com"],
     )
     mockDataInfoContact = fakedEppObject(
         "anotherPw", cr_date=datetime.datetime(2023, 7, 25, 19, 45, 35)
     )
     mockDataInfoHosts = fakedEppObject(
-        "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35), addrs=["1.2.3", "2.3.4"]
+        "lastPw",
+        cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35),
+        addrs=["1.2.3", "2.3.4"],
     )
 
-    mockDataHostChange  =fakedEppObject(
+    mockDataHostChange = fakedEppObject(
         "lastPw", cr_date=datetime.datetime(2023, 8, 25, 19, 45, 35)
     )
+    infoDomainHasIP = fakedEppObject(
+        "nameserverwithip.gov",
+        cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35),
+        contacts=[],
+        hosts=[
+            "ns1.nameserverwithip.gov",
+            "ns2.nameserverwithip.gov",
+            "ns3.nameserverwithip.gov",
+        ],
+    )
 
-    extendedValues=False
+    extendedValues = False
 
     def mockSend(self, _request, cleaned):
         """Mocks the registry.send function used inside of domain.py
@@ -623,10 +639,14 @@ class MockEppLib(TestCase):
             elif getattr(_request, "name", None) == "my-nameserver.gov":
                 if self.extendedValues:
                     return MagicMock(res_data=[self.infoDomainThreeHosts])
-                elif self.mockedSendFunction.call_count==5: ## remove this breaks anything?
+                elif (
+                    self.mockedSendFunction.call_count == 5
+                ):  ## remove this breaks anything?
                     return MagicMock(res_data=[self.infoDomainTwoHosts])
                 else:
                     return MagicMock(res_data=[self.infoDomainNoHost])
+            elif getattr(_request, "name", None) == "nameserverwithip.gov":
+                return MagicMock(res_data=[self.infoDomainHasIP])
             return MagicMock(res_data=[self.mockDataInfoDomain])
         elif isinstance(_request, commands.InfoContact):
             return MagicMock(res_data=[self.mockDataInfoContact])
@@ -638,12 +658,21 @@ class MockEppLib(TestCase):
             # use this for when a contact is being updated
             # sets the second send() to fail
             raise RegistryError(code=ErrorCode.OBJECT_EXISTS)
-        elif (isinstance(_request, commands.CreateHost)):
+        elif isinstance(_request, commands.CreateHost):
-            return MagicMock(res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY)
+            return MagicMock(
-        elif (isinstance(_request, commands.UpdateHost)):
+                res_data=[self.mockDataHostChange],
-            return MagicMock(res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY)
+                code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY,
-        elif (isinstance(_request, commands.DeleteHost)):
+            )
-            return MagicMock(res_data=[self.mockDataHostChange], code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY)
+        elif isinstance(_request, commands.UpdateHost):
+            return MagicMock(
+                res_data=[self.mockDataHostChange],
+                code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY,
+            )
+        elif isinstance(_request, commands.DeleteHost):
+            return MagicMock(
+                res_data=[self.mockDataHostChange],
+                code=ErrorCode.COMMAND_COMPLETED_SUCCESSFULLY,
+            )
         return MagicMock(res_data=[self.mockDataInfoHosts])
 
     def setUp(self):

src/registrar/tests/test_models_domain.py

@@ -537,27 +537,39 @@ class TestRegistrantNameservers(MockEppLib):
         self.nameserver2 = "ns1.my-nameserver-2.com"
         self.nameserver3 = "ns1.cats-are-superior3.com"
 
-        self.domain, _ = Domain.objects.get_or_create(name="my-nameserver.gov", state=Domain.State.DNS_NEEDED)
+        self.domain, _ = Domain.objects.get_or_create(
+            name="my-nameserver.gov", state=Domain.State.DNS_NEEDED
+        )
 
     def test_get_nameserver_changes(self):
-        self.domain._cache["hosts"]=[{
+        self.domain._cache["hosts"] = [
-                        "name": "ns1.example.com",
+            {"name": "ns1.example.com", "addrs": None},
-                        "addrs": None
+            {"name": "ns2.example.com", "addrs": ["1.2.3"]},
-                    },
+            {"name": "ns3.my-nameserver.gov", "addrs": ["1.2.3"]},
-                    {"name": "ns2.example.com",
-                        "addrs": ["1.2.3"]
-                    },
-                    {"name": "ns3.example.com",
-                        "addrs": ["1.2.3"]
-                    }
         ]
-        newChanges=[("ns1.example.com",),("ns3.example.com",["1.2.4"]),("ns4.example.com",)]
+        newChanges = [
-        deleted_values,updated_values,new_values, oldNameservers=self.domain.getNameserverChanges(newChanges)
+            ("ns1.example.com",),
+            ("ns3.my-nameserver.gov", ["1.2.4"]),
+            ("ns4.example.com",),
+        ]
+        (
+            deleted_values,
+            updated_values,
+            new_values,
+            oldNameservers,
+        ) = self.domain.getNameserverChanges(newChanges)
         print(oldNameservers)
-        self.assertEqual(deleted_values, [('ns2.example.com', ['1.2.3'])])
+        self.assertEqual(deleted_values, [("ns2.example.com", ["1.2.3"])])
-        self.assertEqual(updated_values, [('ns3.example.com', ['1.2.4'])])
+        self.assertEqual(updated_values, [("ns3.my-nameserver.gov", ["1.2.4"])])
-        self.assertEqual(new_values, {'ns4.example.com':None})
+        self.assertEqual(new_values, {"ns4.example.com": None})
-        self.assertEqual(oldNameservers, {'ns1.example.com': None, 'ns2.example.com': ['1.2.3'], 'ns3.example.com': ['1.2.3']})
+        self.assertEqual(
+            oldNameservers,
+            {
+                "ns1.example.com": None,
+                "ns2.example.com": ["1.2.3"],
+                "ns3.my-nameserver.gov": ["1.2.3"],
+            },
+        )
 
     def test_user_adds_one_nameserver(self):
         """
@@ -575,7 +587,9 @@ class TestRegistrantNameservers(MockEppLib):
 
         # when you create a host, you also have to update at same time
         created_host = commands.CreateHost(nameserver)
-        update_domain_with_created = commands.UpdateDomain(name=self.domain.name, add=[common.HostObjSet([created_host.name])])
+        update_domain_with_created = commands.UpdateDomain(
+            name=self.domain.name, add=[common.HostObjSet([created_host.name])]
+        )
 
         # checking if commands were sent (commands have to be sent in order)
         expectedCalls = [
@@ -604,12 +618,16 @@ class TestRegistrantNameservers(MockEppLib):
 
         # when you create a host, you also have to update at same time
         created_host1 = commands.CreateHost(self.nameserver1)
-        update_domain_with_created1 = commands.UpdateDomain(name=self.domain.name, add=[common.HostObjSet([created_host1.name])])
+        update_domain_with_created1 = commands.UpdateDomain(
+            name=self.domain.name, add=[common.HostObjSet([created_host1.name])]
+        )
 
         created_host2 = commands.CreateHost(self.nameserver2)
-        update_domain_with_created2 = commands.UpdateDomain(name=self.domain.name, add=[common.HostObjSet([created_host2.name])])
+        update_domain_with_created2 = commands.UpdateDomain(
+            name=self.domain.name, add=[common.HostObjSet([created_host2.name])]
+        )
 
-        infoDomain = commands.InfoDomain(name='my-nameserver.gov', auth_info=None)
+        infoDomain = commands.InfoDomain(name="my-nameserver.gov", auth_info=None)
         # checking if commands were sent (commands have to be sent in order)
         expectedCalls = [
             call(infoDomain, cleaned=True),
@@ -619,9 +637,6 @@ class TestRegistrantNameservers(MockEppLib):
             call(update_domain_with_created2, cleaned=True),
         ]
 
-        print("self.mockedSendFunction.call_args_list is ")
-        print(self.mockedSendFunction.call_args_list)
-
         self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True)
         self.assertEqual(5, self.mockedSendFunction.call_count)
         # check that status is READY
@@ -652,9 +667,22 @@ class TestRegistrantNameservers(MockEppLib):
         nameserver14 = "ns1.cats-are-superior14.com"
 
         def _get_14_nameservers():
-            self.domain.nameservers = [(nameserver1,), (nameserver2,), (nameserver3,), (nameserver4,), 
+            self.domain.nameservers = [
-        (nameserver5,), (nameserver6,), (nameserver7,), (nameserver8,), (nameserver9), (nameserver10,),
+                (nameserver1,),
-        (nameserver11,), (nameserver12,), (nameserver13,), (nameserver14,)]
+                (nameserver2,),
+                (nameserver3,),
+                (nameserver4,),
+                (nameserver5,),
+                (nameserver6,),
+                (nameserver7,),
+                (nameserver8,),
+                (nameserver9),
+                (nameserver10,),
+                (nameserver11,),
+                (nameserver12,),
+                (nameserver13,),
+                (nameserver14,),
+            ]
 
         self.assertRaises(ValueError, _get_14_nameservers)
         self.assertEqual(self.mockedSendFunction.call_count, 0)
@@ -670,25 +698,37 @@ class TestRegistrantNameservers(MockEppLib):
         """
 
         # Mock is set to return 3 nameservers on infodomain
-        self.extendedValues=True
+        self.extendedValues = True
         self.domain.nameservers = [(self.nameserver1,), (self.nameserver2,)]
-        expectedCalls=[  
+        expectedCalls = [
             # calls info domain, and info on all hosts
             # to get past values
             # then removes the single host and updates domain
-            call(commands.InfoDomain(name='my-nameserver.gov', auth_info=None), cleaned=True),
+            call(
-            call(commands.InfoHost(name='ns1.my-nameserver-1.com'), cleaned=True),
+                commands.InfoDomain(name="my-nameserver.gov", auth_info=None),
-            call(commands.InfoHost(name='ns1.my-nameserver-2.com'), cleaned=True),
+                cleaned=True,
-            call(commands.InfoHost(name='ns1.cats-are-superior3.com'), cleaned=True),
+            ),
-            call(commands.UpdateDomain(name='my-nameserver.gov', add=[], rem=[common.HostObjSet(hosts=['ns1.cats-are-superior3.com'])], nsset=None, keyset=None, registrant=None, auth_info=None), cleaned=True),
+            call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True),
-            call(commands.DeleteHost(name='ns1.cats-are-superior3.com'), cleaned=True)]
+            call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True),
+            call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True),
+            call(
+                commands.UpdateDomain(
+                    name="my-nameserver.gov",
+                    add=[],
+                    rem=[common.HostObjSet(hosts=["ns1.cats-are-superior3.com"])],
+                    nsset=None,
+                    keyset=None,
+                    registrant=None,
+                    auth_info=None,
+                ),
+                cleaned=True,
+            ),
+            call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True),
+        ]
 
-        print("self.mockedSendFunction.call_args_list is ")
-        print(self.mockedSendFunction.call_args_list)
         self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True)
         self.assertTrue(self.domain.is_active())
 
-  
     def test_user_removes_too_many_nameservers(self):
         """
         Scenario: Registrant removes some nameservers, bringing the total to less than 2
@@ -699,24 +739,48 @@ class TestRegistrantNameservers(MockEppLib):
             And `domain.is_active` returns False
 
         """
-        self.extendedValues=True
+        self.extendedValues = True
         print("domain state")
         print(self.domain.state)
         self.domain.ready()
         print("Domain state is now")
         print(self.domain.state)
         self.domain.nameservers = [(self.nameserver1,)]
-        print("self.mockedSendFunction.call_args_list is ")
+        expectedCalls = [
-        print(self.mockedSendFunction.call_args_list)
+            call(
-        expectedCalls=[call(commands.InfoDomain(name='my-nameserver.gov', auth_info=None), cleaned=True),
+                commands.InfoDomain(name="my-nameserver.gov", auth_info=None),
-            call(commands.InfoHost(name='ns1.my-nameserver-1.com'), cleaned=True),
+                cleaned=True,
-            call(commands.InfoHost(name='ns1.my-nameserver-2.com'), cleaned=True),
+            ),
-            call(commands.InfoHost(name='ns1.cats-are-superior3.com'), cleaned=True),
+            call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True),
-            call(commands.UpdateDomain(name='my-nameserver.gov', add=[], rem=[common.HostObjSet(hosts=['ns1.my-nameserver-2.com'])], nsset=None, keyset=None, registrant=None, auth_info=None), cleaned=True),
+            call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True),
-            call(commands.DeleteHost(name='ns1.my-nameserver-2.com'), cleaned=True),
+            call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True),
-            call(commands.UpdateDomain(name='my-nameserver.gov', add=[], rem=[common.HostObjSet(hosts=['ns1.cats-are-superior3.com'])], nsset=None, keyset=None, registrant=None, auth_info=None), cleaned=True),
+            call(
-            call(commands.DeleteHost(name='ns1.cats-are-superior3.com'), cleaned=True)]
+                commands.UpdateDomain(
-        
+                    name="my-nameserver.gov",
+                    add=[],
+                    rem=[common.HostObjSet(hosts=["ns1.my-nameserver-2.com"])],
+                    nsset=None,
+                    keyset=None,
+                    registrant=None,
+                    auth_info=None,
+                ),
+                cleaned=True,
+            ),
+            call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True),
+            call(
+                commands.UpdateDomain(
+                    name="my-nameserver.gov",
+                    add=[],
+                    rem=[common.HostObjSet(hosts=["ns1.cats-are-superior3.com"])],
+                    nsset=None,
+                    keyset=None,
+                    registrant=None,
+                    auth_info=None,
+                ),
+                cleaned=True,
+            ),
+            call(commands.DeleteHost(name="ns1.cats-are-superior3.com"), cleaned=True),
+        ]
 
         self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True)
         self.assertFalse(self.domain.is_active())
@@ -731,27 +795,72 @@ class TestRegistrantNameservers(MockEppLib):
             And `commands.UpdateDomain` is sent to add #4 and #5 plus remove #2 and #3
             And `commands.DeleteHost` is sent to delete #2 and #3
         """
-        self.extendedValues=True
+        self.extendedValues = True
         self.domain.ready()
-        self.domain.nameservers=[(self.nameserver1,), ("ns1.cats-are-superior1.com",), ("ns1.cats-are-superior2.com",)]
+        self.domain.nameservers = [
-        print("self.mockedSendFunction.call_args_list is ")
+            (self.nameserver1,),
-        print(self.mockedSendFunction.call_args_list)
+            ("ns1.cats-are-superior1.com",),
-        expectedCalls=[call(commands.InfoDomain(name='my-nameserver.gov', auth_info=None), cleaned=True),
+            ("ns1.cats-are-superior2.com",),
-            call(commands.InfoHost(name='ns1.my-nameserver-1.com'), cleaned=True),
+        ]
-            call(commands.InfoHost(name='ns1.my-nameserver-2.com'), cleaned=True),
+
-            call(commands.InfoHost(name='ns1.cats-are-superior3.com'), cleaned=True),
+        expectedCalls = [
-            call(commands.UpdateDomain(name='my-nameserver.gov', add=[], rem=[common.HostObjSet(hosts=['ns1.my-nameserver-2.com'])], nsset=None, keyset=None, registrant=None, auth_info=None), cleaned=True),
+            call(
-            call(commands.DeleteHost(name='ns1.my-nameserver-2.com'), cleaned=True),
+                commands.InfoDomain(name="my-nameserver.gov", auth_info=None),
-            call(commands.CreateHost(name='ns1.cats-are-superior1.com', addrs=[]), cleaned=True),
+                cleaned=True,
-            call(commands.UpdateDomain(name='my-nameserver.gov', add=[common.HostObjSet(hosts=['ns1.cats-are-superior1.com'])], rem=[], nsset=None, keyset=None, registrant=None, auth_info=None), cleaned=True),
+            ),
-            call(commands.CreateHost(name='ns1.cats-are-superior2.com', addrs=[]), cleaned=True),
+            call(commands.InfoHost(name="ns1.my-nameserver-1.com"), cleaned=True),
-            call(commands.UpdateDomain(name='my-nameserver.gov', add=[common.HostObjSet(hosts=['ns1.cats-are-superior2.com'])], rem=[], nsset=None, keyset=None, registrant=None, auth_info=None), cleaned=True)
+            call(commands.InfoHost(name="ns1.my-nameserver-2.com"), cleaned=True),
+            call(commands.InfoHost(name="ns1.cats-are-superior3.com"), cleaned=True),
+            call(
+                commands.UpdateDomain(
+                    name="my-nameserver.gov",
+                    add=[],
+                    rem=[common.HostObjSet(hosts=["ns1.my-nameserver-2.com"])],
+                    nsset=None,
+                    keyset=None,
+                    registrant=None,
+                    auth_info=None,
+                ),
+                cleaned=True,
+            ),
+            call(commands.DeleteHost(name="ns1.my-nameserver-2.com"), cleaned=True),
+            call(
+                commands.CreateHost(name="ns1.cats-are-superior1.com", addrs=[]),
+                cleaned=True,
+            ),
+            call(
+                commands.UpdateDomain(
+                    name="my-nameserver.gov",
+                    add=[common.HostObjSet(hosts=["ns1.cats-are-superior1.com"])],
+                    rem=[],
+                    nsset=None,
+                    keyset=None,
+                    registrant=None,
+                    auth_info=None,
+                ),
+                cleaned=True,
+            ),
+            call(
+                commands.CreateHost(name="ns1.cats-are-superior2.com", addrs=[]),
+                cleaned=True,
+            ),
+            call(
+                commands.UpdateDomain(
+                    name="my-nameserver.gov",
+                    add=[common.HostObjSet(hosts=["ns1.cats-are-superior2.com"])],
+                    rem=[],
+                    nsset=None,
+                    keyset=None,
+                    registrant=None,
+                    auth_info=None,
+                ),
+                cleaned=True,
+            ),
         ]
 
         self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True)
         self.assertTrue(self.domain.is_active())
 
-
     def test_user_cannot_add_subordinate_without_ip(self):
         """
         Scenario: Registrant adds a nameserver which is a subdomain of their .gov
@@ -760,11 +869,12 @@ class TestRegistrantNameservers(MockEppLib):
                 with a subdomain of the domain and no IP addresses
             Then Domain raises a user-friendly error
         """
-        #add a nameserver with a .gov and no ip
-        ##assertRaises error 
-        raise
 
-    @skip("not implemented yet")
+        dotgovnameserver = "my-nameserver.gov"
+
+        with self.assertRaises(ValueError):
+            self.domain.nameservers = [(dotgovnameserver,)]
+
     def test_user_updates_ips(self):
         """
         Scenario: Registrant changes IP addresses for a nameserver
@@ -774,9 +884,49 @@ class TestRegistrantNameservers(MockEppLib):
                 with a different IP address(es)
             Then `commands.UpdateHost` is sent to the registry
         """
-        raise
+        domain, _ = Domain.objects.get_or_create(
+            name="nameserverwithip.gov", state=Domain.State.READY
+        )
+        domain.nameservers = [
+            ("ns1.nameserverwithip.gov", ["2.3.4", "1.2.3"]),
+            ("ns2.nameserverwithip.gov", ["1.2.3", "2.3.4", "3.4.5"]),
+            ("ns3.nameserverwithip.gov", ["2.3.4"]),
+        ]
+
+        # print("self.mockedSendFunction.call_args_list is ")
+        # print(self.mockedSendFunction.call_args_list)
+
+        expectedCalls = [
+            call(
+                commands.InfoDomain(name="nameserverwithip.gov", auth_info=None),
+                cleaned=True,
+            ),
+            call(commands.InfoHost(name="ns1.nameserverwithip.gov"), cleaned=True),
+            call(commands.InfoHost(name="ns2.nameserverwithip.gov"), cleaned=True),
+            call(commands.InfoHost(name="ns3.nameserverwithip.gov"), cleaned=True),
+            call(
+                commands.UpdateHost(
+                    name="ns2.nameserverwithip.gov",
+                    add=[common.Ip(addr="3.4.5", ip="v6")],
+                    rem=[],
+                    chg=None,
+                ),
+                cleaned=True,
+            ),
+            call(
+                commands.UpdateHost(
+                    name="ns3.nameserverwithip.gov",
+                    add=[],
+                    rem=[common.Ip(addr="1.2.3", ip="v6")],
+                    chg=None,
+                ),
+                cleaned=True,
+            ),
+        ]
+
+        self.mockedSendFunction.assert_has_calls(expectedCalls, any_order=True)
+        self.assertTrue(domain.is_active())
 
-    @skip("not implemented yet")
     def test_user_cannot_add_non_subordinate_with_ip(self):
         """
         Scenario: Registrant adds a nameserver which is NOT a subdomain of their .gov
@@ -785,7 +935,10 @@ class TestRegistrantNameservers(MockEppLib):
                 which is not a subdomain of the domain and has IP addresses
             Then Domain raises a user-friendly error
         """
-        raise
+        dotgovnameserver = "mynameserverdotgov.gov"
+
+        with self.assertRaises(ValueError):
+            self.domain.nameservers = [(dotgovnameserver, ["1.2.3"])]
 
     @skip("not implemented yet")
     def test_nameservers_are_idempotent(self):
@@ -810,9 +963,10 @@ class TestRegistrantNameservers(MockEppLib):
         raise
 
     def tearDown(self):
-        self.extendedValues=False
+        self.extendedValues = False
         return super().tearDown()
 
+
 class TestRegistrantDNSSEC(TestCase):
     """Rule: Registrants may modify their secure DNS data"""