{"action":"created","issue":{"url":"https://api.github.com/repos/samtools/samtools/issues/410","labels_url":"https://api.github.com/repos/samtools/samtools/issues/410/labels{/name}","comments_url":"https://api.github.com/repos/samtools/samtools/issues/410/comments","events_url":"https://api.github.com/repos/samtools/samtools/issues/410/events","html_url":"https://github.com/samtools/samtools/issues/410","id":88540335,"number":410,"title":"mpileup test fails in non-English locale","user":{"login":"charles-plessy","id":1834905,"avatar_url":"https://avatars.githubusercontent.com/u/1834905?v=3","gravatar_id":"","url":"https://api.github.com/users/charles-plessy","html_url":"https://github.com/charles-plessy","followers_url":"https://api.github.com/users/charles-plessy/followers","following_url":"https://api.github.com/users/charles-plessy/following{/other_user}","gists_url":"https://api.github.com/users/charles-plessy/gists{/gist_id}","starred_url":"https://api.github.com/users/charles-plessy/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/charles-plessy/subscriptions","organizations_url":"https://api.github.com/users/charles-plessy/orgs","repos_url":"https://api.github.com/users/charles-plessy/repos","events_url":"https://api.github.com/users/charles-plessy/events{/privacy}","received_events_url":"https://api.github.com/users/charles-plessy/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-06-15T21:02:19Z","updated_at":"2015-06-30T11:21:11Z","closed_at":null,"body":"Hello,\r\n\r\nthis is quite minor, but the mpileup test fails in non-English locales:\r\n\r\n```\r\nUNEXPECTED FAIL: Output mismatch for $samtools mpileup -u -x -f mpileup.ref.fa mpileup.1.bam | GREP_OPTIONS= egrep -v '^##samtools'\r\nSee FAIL-52.out.1 vs expected/52.out\r\n\r\n$ cat expected/52.out\r\nBinary file (standard input) matches\r\n\r\n$ cat FAIL-52.out.1 \r\nFichier binaire (entrée standard) correspondant\r\n```\r\n\r\nObviously, the locale should be set to `C` somewhere, but I am not sure where it would be most appropriate; sorry for not proposing a patch. Adding `LC_ALL=C` in front of `egrep` would solve this problem but would not prevent it from happening somewhere else…\r\n\r\n-- \r\nCharles"},"comment":{"url":"https://api.github.com/repos/samtools/samtools/issues/comments/117129233","html_url":"https://github.com/samtools/samtools/issues/410#issuecomment-117129233","issue_url":"https://api.github.com/repos/samtools/samtools/issues/410","id":117129233,"user":{"login":"jkbonfield","id":2210525,"avatar_url":"https://avatars.githubusercontent.com/u/2210525?v=3","gravatar_id":"","url":"https://api.github.com/users/jkbonfield","html_url":"https://github.com/jkbonfield","followers_url":"https://api.github.com/users/jkbonfield/followers","following_url":"https://api.github.com/users/jkbonfield/following{/other_user}","gists_url":"https://api.github.com/users/jkbonfield/gists{/gist_id}","starred_url":"https://api.github.com/users/jkbonfield/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jkbonfield/subscriptions","organizations_url":"https://api.github.com/users/jkbonfield/orgs","repos_url":"https://api.github.com/users/jkbonfield/repos","events_url":"https://api.github.com/users/jkbonfield/events{/privacy}","received_events_url":"https://api.github.com/users/jkbonfield/received_events","type":"User","site_admin":false},"created_at":"2015-06-30T11:21:11Z","updated_at":"2015-06-30T11:21:11Z","body":"On Tue, Jun 30, 2015 at 02:47:29AM -0700, John Marshall wrote:\n> What is this checking for anyway?  Non-empty output (modulo a `##samtools` header)?  Could we use `wc` (or even `strings`+`grep`) instead?\n\nHmm I see your point - it's a borked test basically.\n\nWhen I did the testing of mpileup it was largely as a naive uneducated\nuser of mpileup.  Infact there are still mpileup functionality that I\ndon't understand!  My aim, and indeed the groups aim, was to checking\nthat the new samtools 1.0 mpileup worked the same as the old samtools\n0.1.19 mpileup so we could spot any breakages by our grand\nreorganisations; hence calling it a regression test rather than a\nfunctionality test.  Therefore my expected outputs were basically\ngenerated by running my battery of tests with the old samtools so I\ncould validate the new one gives the same output.  (Although I did\nfile numerous bugs I discovered on the way too.)\n\nWhat it was attempting to check was that the bcf file generated is\ncorrect, minus the ##samtools line which differs on each and every\nversion and so would break regression testing.\n\nWhat it actually did without me noticing somehow (sorry) was convert\nall that to a meaningless \"binary file matches\" statement.  I think\nwhen I initially wrote it it was using bcftools view, but that adds\nannoying dependencies between packages so it got hacked to a grep,\nbadly.\n\nA ...|sed 's/^##samtools//' seems to work, but I'm not sure if that is\ngoing to trip us up elsewhere too.  Note that strings wouldn't work as\nit can only check the text portions.\n\nJames\n\nPS.  The BCF format has radically changed between 0.1.19 and 1.2\nanyway, so this would have broken ages ago, possible multiple times.\nI think it's still a valid test to have though, even if we have to\nupdate the expected output periodically.  Alternatively if Petr\nbelieves the VCF to BCF conversion is adequately tested in bcftools or\nin htslib then we can skip this test here and only check that VCF\noutput works.\n\n-- \nJames Bonfield (jkb@sanger.ac.uk) | Hora aderat briligi. Nunc et Slythia Tova\n                                  | Plurima gyrabant gymbolitare vabo;\n  A Staden Package developer:     | Et Borogovorum mimzebant undique formae,\nhttps://sf.net/projects/staden/   | Momiferique omnes exgrabure Rathi. \n\n\n-- \n The Wellcome Trust Sanger Institute is operated by Genome Research \n Limited, a charity registered in England with number 1021457 and a \n company registered in England with number 2742969, whose registered \n office is 215 Euston Road, London, NW1 2BE. \n"}}