CircleCI-Public / circleci-demo-elixir-phoenix

A CircleCI demo project using Elixir and Phoenix
https://circleci.com/gh/CircleCI-Public/circleci-demo-elixir-phoenix
11 stars 24 forks source link

CircleCI `--split-by=timings` issue with Elixir's junit-formatter #9

Open danadaldos opened 4 years ago

danadaldos commented 4 years ago

Hey all,

I'm wanting to signal-boost an issue I have reported on CircleCI's forums (https://discuss.circleci.com/t/split-by-timings-on-elixir-project-fails-to-find-stored-timings-possibly-missing-file-names-in-junit-xml/34813)

Long story short, with the understanding that this demo project is NOT using the split-by=timings it seems like Elixir's junit-formatter does not include a file or filename attribute that other JUnit formatters for other languages do, and that that attribute might be required by CircleCI's split --split-by=timings utility.

Here is a copy of the forum post:

Hey all,

I am trying to get --split-by=timings to work on an Elixir project but I keep getting the following error:

$ TESTFILES=$(circleci tests glob "test/**/*_test.exs" | circleci tests split --split-by=timings --show-counts)

Read 359 lines of autodetect(s)
Error autodetecting timing type, falling back to weighting by name. Autodetect no matching filename or classname.  If file names are used, double check paths for absolute vs relative.
Example input file: "test/channels/filter_header_channel_test.exs"
Example file from timings: ""
Bucket 0: assigning 90 autodetect(s), total weight 90

When I SSH into a container, here's what the very end of .circleci-task-data/circle-test-results/results.json look like:

{"classname":"Elixir.Foo.ControllerTest","file":null,"name":"test some stuff","result":"success","run_time":0.5231,"message":null,"source":"unknown","source_type":"unknown"}
{"classname":"Elixir.FooProject.SomeTest","file":null,"name":"test some stuff","result":"success","run_time":0.0038,"message":null,"source":"unknown","source_type":"unknown"}],

"files":["results.xml","results.xml","results.xml","results.xml"],"exceptions":[]}

Note that all of the test cases in that file have "file":null. ("files" seems to be an aggregation from the 4 containers I'm running (parallelism: 4)

I have compared notes with someone running a Ruby project, and the JSON in his test container looks like:

{
  "tests": [
    {"classname": "...", "file": "ACTUAL/FILE/NAME", "name": "...", "result": "...", "run_time": "...", "message": "...", "source": "...", "source_type": "..."},
    ...
  ]
}

And, as noted in the sample, all of his test cases have a file name.

Now, I assume that results.json is created out of the JUnit formatter results that get saved as part of the store_test_results step in the build. Looking into the XML that gets stored after a build, and comparing notes again with the Ruby project, it appears that for Ruby/Rspec projects, RspecJunitFormatter outputs XML that includes a file attribute, whereas the XML outputted from Elixir's JUnit Formatter (https://github.com/victorolinasc/junit-formatter) does NOT have that field. Compare:

rspec.xml

<testsuite>
    <testcase classname="..." name="..." file="..." time="..." />
</testsuite>

VS.

exunit.xml

<testsuite>
    <testcase classname="Elixir.FooProject.SomeQueryTest" name="test where stuff happens" time="0.1038"/>
</testsuite>

Okay, so one includes file names, the other does not. CircleCI's split function has a --timings-type flag that can be passed-in to tell the splitter to either look for filename or classname. However, adding --timings-type=classname to the command does not fix the issue, instead, it causes a new error:

Read 359 lines of classname(s)
No timing found for "test/channels/test1.exs"
No timing found for "test/channels/test2.exs"
...
No timing found for "test/views/test200.exs"
No timing found for "test/views/test201.exs"
Bucket 0: assigning 90 classname(s), total weight 2799000 

Which leads me to believe that CircleCI's split utility requires a file name in the JUnit XML, which currently is not available in Elixir's junit-formatter.

Can someone confirm this or not? Am I missing something in my configuration?


Config.yml

For thoroughness, here are the relevant parts of my config file:

version: 2.1
jobs:
  build:
    docker:
      - image: circleci/elixir:1.8.2-browsers
        environment:
          MIX_ENV: test
      - image: redis:4.0.14
      - image: circleci/postgres:10.10-alpine-postgis
        environment:
          POSTGRES_USER: postgres
          POSTGRES_PASSWORD:
    working_directory: /home/circleci/my_project
    parallelism: 4
    steps:
      - checkout

      ... caching stuff ...

      - run: TESTFILES=$(circleci tests glob "test/**/*_test.exs" | circleci tests split --split- 
             by=timings --timings-type=classname --show-counts)

      - run: mix test ${TESTFILES} --trace

      - store_test_results:
          path: test-results/exunit

Other knowns:

Additionally, I would draw your attention to a comment made in the RspecJunitFormatter repo: https://github.com/sj26/rspec_junit_formatter/pull/70

Specifically the comment:

I think both file and filename are non-standard. Junit is a pretty lean, and isn't really standardised at all, it's just what the original junit tool used to produce. But many tools consuming rspec junit formatter output now rely on the file attribute so we cannot remove it now. I'm afraid this might be a problem xunit needs to fix — to ignore unknown attributes, or something.

zzak commented 4 years ago

@danadaldos Thanks for your thorough report and investigation.

Without going deeply into the circleci tests command, it does seem odd that the file name is missing from the data and likely to be required. I wonder if it's possible to create a smaller demonstration of this bug with just Elixir's junit-formatter library, then if there isn't already been a fix in that library or at least a known issue.

My hunch is that this problem occurs between the formatter library being used here, and not the CLI. But I'm open to keeping this issue around in case anyone else has ideas.

Thanks again! :heart:

danadaldos commented 4 years ago

@zzak I forked the junit-formatter library and added the necessary file attribute to the <testcase>s and it did fix the timings issue. I opened a PR on that library in order to allow people to configure junit-formatter to include the file name.

I didn't want this to be an intrusive addition to junit-formatter, so it is an optional configuration that defaults to false. If that gets merged into the library, maybe we can communicate on this demo app that split-by=timings on Elixir needs the junit-formatter and to have it configured to include_filename?: true. I'll follow up when I hear back from the maintainers of that library.

Thank you!

zzak commented 4 years ago

@danadaldos Thanks so much for your contribution!

I'll follow up when I hear back from the maintainers of that library.

Sounds good, please feel free anytime if you'd like to add or change anything in our example apps or docs.

danadaldos commented 4 years ago

@zzak Okay, the owner of junit-formatter merged the PR, so now if you configure that library with include_filename?: true it will produce XML that has the filename and allows CircleCI to split-by=timings. I created a documentation PR that explains these changes and how Elixir + CircleCI projects need to be configured.