Code review comment for lp:~dimitern/maas/bug-994887-alternate-nodes-list-sorting

        # Just creare some nodes to have something in the list

Tyop, 'create'. Hm, we should really make a helper for multi-node creation on these views, but that can happen later.

+ sorted_nodes = sorted(nodes, key=lambda x: x.hostname)

Always nice to use operator.attrgetter('hostname') for this.

+ header_links = document.xpath("//div[@id='nodes']/table//th/a/@href")
+ for link in header_links:
+ self.assertIn('page=1', link)
+ self.assertIn('query=maas-tags%3Dshiny', link)

Rather than having multiple string contains assertions like this, I'd use urlparse.parse_qsl and testtools matchers against all the key/values you expect, so a failure has the full context. Something like:

    self.assertThat(urlparse.parse_qsl(link), ContainsAll([('page', '1'), ...]))

- node_links = document.xpath("//div[@id='nodes']/table//a/@href")
+ # Exclude header sorting links
+ node_links = filter(
+ lambda x: 'sort=' not in x,
+ document.xpath("//div[@id='nodes']/table//a/@href"))

For these, I'd adjust the xpath instead, ie, .../table//td//a/...

+ links = {field: self.request.path + '?' for field in fields}

Give relative links rather than including the path in them, the link should just start with the '?', note tests will need changing to expect this too.