Testing done #138
Testing done #138
Conversation
| if node_status.dockerRunning: | ||
| if request.method == api_constants.MGMT_WEBAPP.HTTP_REST_POST: | ||
| if node.ip == ip: | ||
| # what about node.ip != ip? |
There was a problem hiding this comment.
Du kan lägga in en boolean variable och kolla om ip inte matchar någon node.ip kan du returnera return jsonify({"reason": f"node with ip {ip} does not exist."}), constants.HTTPS.BAD_REQUEST_STATUS_CODE
Om det används i två testfiler är det värt att flytta den till conftest tycker jag, du kan göra den refaktoreringen som en del i denna PR |
Fixat |
| if node.ip == ip: | ||
| if request.method == api_constants.MGMT_WEBAPP.HTTP_REST_POST: | ||
| if node_status.dockerRunning: | ||
| if request.method == api_constants.MGMT_WEBAPP.HTTP_REST_POST: |
There was a problem hiding this comment.
Jag tror inte detta är korrekt. Säg att jag har två cluster nodes med id 1 och 2. Antag att jag gör ett POST request med id=2. Då kommer jag få en BAD REQUEST pga att 1 != 2 (Rätta mig om jag har fel).
Jag tror du måste lägga in en Boolean variabel innan loopen börjar, ex ip_found=False, dvs innan for node in config.cluster_config.cluster_nodes: sedan om du får en match på IP inuti loopen då sätter du ip_found = True. Sedan har du en if-statement utanför loopen:
if not ip_found: return BAD_REQUEST`
There was a problem hiding this comment.
EDIT: På if-statement utanför loopen måste du också kolla att det är en POST. i.e. if not ip_found and method==POST: return BAD REQUEST
There was a problem hiding this comment.
Oright! Ska först göra klart get-metoden i experiments, sen fixar jag detta
There was a problem hiding this comment.
Jag gjorde ett förslag på lösning med ip_found. Vet inte om du ville att jag skulle byta plats på POST-statement, men tidigare då ip har påkallats före POST har vi haft en bugg p.g.a att GET inte har någon ip och då fås "referenced before assignment" för samtliga GET-requests.
| for node in config.cluster_config.cluster_nodes: | ||
| ip_found = False |
There was a problem hiding this comment.
Denna raden kan tas bort. Det räcker med att 1 ip i cluster node matchar så ska ip_found vara True. I nuläget om vi har 2 cluster nodes och första ip matchar men andra matchar inte så får vi BAD_REQUEST pga den raden
There was a problem hiding this comment.
Oh missade att ta bort den, fixat!
En del buggar upptäckte i routes tror jag.
Ska jag göra någon testkod ifall node.ip != ip? Jag tänker att isåfall borde testingen för get vara tillräcklig men inte säker. Har markerat var jag menar i routes med en inline-kommentar som jag tar bort sen.
Värt att lägga in cluster_node_status i conftest? Just nu använder jag den i två test-filer och kan tänka mig att den förekommer fler gånger framöver(?).