How would removing or modifying the test change our response to a user saying "Sorting doesn't look right"?
-Zach On Mon, Jun 21, 2021 at 3:48 PM ocket 8888 <[email protected]> wrote: > But if we test our sorting according to a certain locale, our response to > "sorting doesn't look right" is going to be "what's your locale?" instead > of "what's wrong?". If the behavior as tested only works in one locale, > that's the only locale it supports. This is why I think tests are a > statement of intent about the behavior under test. I'm fine with a weaker > test if it better represents what we actually want. > > On Mon, Jun 21, 2021 at 1:46 PM Zach Hoffman <[email protected]> wrote: > > > > I don't feel good about telling the community/world "this software only > > runs on machines configured for United States-flavor English". > > > > We shouldn't need to tell the community/world "this software only runs on > > machines configured for United States-flavor English," we can just > document > > that a specific locale is required for running the API tests. > > > > -Zach > > > > On Mon, Jun 21, 2021 at 1:40 PM ocket 8888 <[email protected]> wrote: > > > > > > I think the likelihood of that test passing when underlying > > functionality > > > *IS BROKEN* can be made acceptably low... > > > > > > clarification > > > > > > On Mon, Jun 21, 2021 at 1:35 PM ocket 8888 <[email protected]> > wrote: > > > > > > > Currently, as some of you may have noticed, the integration tests for > > > > Traffic Ops and its Go API client are failing on master. This is > > > > (partially) because of a test that checks that the response to a GET > > > > request to /deliveryservices at version 4.0 is sorted by XMLID if no > > > > orderby query string parameter is provided. The way it does this is > by > > > > making the request, then sorting the returned XMLIDs, then comparing > > the > > > > actual response to the sorted list. > > > > > > > > Here's the problem: the ordering is done in an SQL "ORDER BY" clause, > > > > which - besides being a different runtime - runs on ostensibly an > > > entirely > > > > separate system from the client/TO integration tests. If those two > > > systems > > > > aren't using the same locale, they won't sort the same set of strings > > in > > > > the same way. Specifically, right now our tests use a C locale, which > > > sorts > > > > "-" *after* "1", but the postgresql service uses en_US.utf8, which > > sorts > > > > "-" *before* "1". > > > > > > > > This is the tests passing (that one, specifically, some vault things > > are > > > > still failing) after add a `COLLATE "C"` statement to the query's > > "ORDER > > > > BY" clause: > > > > > > > > > > https://github.com/apache/trafficcontrol/pull/5957/checks?check_run_id=2878328870 > > > > > > > > This is the tests failing without that extra statement: > > > > https://github.com/apache/trafficcontrol/runs/2853688690 > > > > > > > > Now: why is this a mailing list matter? Because, all of our laptops > are > > > > using en_US.utf8, or somebody would've said something way earlier. > And > > > > every attempt I've made (in this PR: > > > > https://github.com/apache/trafficcontrol/pull/5957) to change the > > locale > > > > in which the tests are running has failed. So adding that locale > > > statement > > > > will cause the tests to start failing without extra configuration on > > > > everyone's local development machines and in every existing CI > > > environment > > > > besides GHA, and we apparently can't force the GHA tests to comply > with > > > our > > > > existing environments (I think - if I've missed something somebody > > please > > > > let me know). > > > > > > > > To solve this, I can think of three options: > > > > > > > > 1. Get rid of the test. Default ordering is undocumented, so nobody > > > should > > > > actually be relying on it. > > > > 2. Change the test; the original purpose behind default ordering was > to > > > > make requests deterministic so that with the same data in TO you > would > > > > always get the same response. The test can't be absolutely sure that > if > > > it > > > > checks N times that if it did it N+1 times the consistency wouldn't > be > > > > broken, but we can ensure that under testing conditions that making, > > > say, 3 > > > > identical, subsequent requests with identical data in TO yields > > identical > > > > responses. It's a weaker test, but it could be said it's better than > > > > nothing. > > > > 3. Enforce a locale. If the actual order is important to us, then we > > must > > > > enforce a locale, because that defines the order. en_US.UTF-8 seems a > > > > logical choice, so that the only thing that needs to change is the > > test. > > > It > > > > will mean, materially, writing our own sorting function that uses > > > > en_US.UTF-8 rules to compare bytes in a string - regardless of the > > > > execution environment's locale - and adding some documentation that > > notes > > > > that our supported environments are restricted to a specific locale. > > > > > > > > Personally, I like option 2. I think our tests ought to reflect the > > > > intended behavior, not implementation details. I think the likelihood > > of > > > > that test passing when underlying functionality can be made > acceptably > > > low, > > > > and I think that in general the locale of a system is not something a > > > > single piece of software running on it should dictate. I don't feel > > good > > > > about telling the community/world "this software only runs on > machines > > > > configured for United States-flavor English". > > > > > > > > > >
