Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally generate build_report.json #38

Merged

Conversation

hoyon
Copy link
Contributor

@hoyon hoyon commented May 31, 2023

Adds a --json-report flag which can be used to generate a json report which looks something like this:

{
  "steps": [
    {
      "build_step_name": "MIX_ENV=dev mix compile",
      "duration_in_microseconds": 627731,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "MIX_ENV=dev mix escript.build",
      "duration_in_microseconds": 790919,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "MIX_ENV=test mix compile --force --warnings-as-errors",
      "duration_in_microseconds": 1082262,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "end_to_end_test",
      "duration_in_microseconds": 1194515,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "escriptBuild",
      "duration_in_microseconds": 900716,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "exit_code_correctness_end_to_end_test",
      "duration_in_microseconds": 301516,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "find_todos",
      "duration_in_microseconds": 19252,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "mix deps.get",
      "duration_in_microseconds": 1161638,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "mix loadconfig config/prod.exs",
      "duration_in_microseconds": 620232,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "mix test --color",
      "duration_in_microseconds": 1286466,
      "exit_code": 0,
      "status": "complete"
    },
    {
      "build_step_name": "preflight_checks_return_non_zero_exit_code_test",
      "duration_in_microseconds": 232673,
      "exit_code": 0,
      "status": "complete"
    }
  ]
}

Test by running:

  1. MIX_ENV=prod mix escript.build
  2. ./bp run --json-report
  3. cat build_report.json

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #27167: Extend build pipeline to gather stats on failure reason.

@hoyon hoyon requested a review from mbernerslee May 31, 2023 14:27
Copy link
Contributor

@mbernerslee mbernerslee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some more tests perhaps?

--ra
--stats
"""
def incompatible_args_error do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json-report cannot be set with debug.
say that here

@@ -0,0 +1,17 @@
defmodule BuildPipeline.Run.JsonReport do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

write a test that at this level that asserts a file gets written?
use Mimic to not actually write a file?

I'm open to you saying "nah, i dont think its needed"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but thats what I'd do I think

@@ -123,6 +135,10 @@ defmodule BuildPipeline.Run.CommandLineArguments do
{:ok, Map.put(setup, :show_stats, true)}
end

defp put_setup_from_singular_cli_arg(setup, @json_report) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've TDD'd adding a new command line arg by adding a high level test
either at the server_test or run_test (cant remember which) level.

Add such a high level test for json report?

I'm open to pushback on this, but that is how I added all the other CLI args anyhow

@hoyon hoyon requested a review from mbernerslee June 1, 2023 10:26
@mbernerslee mbernerslee merged commit d90c937 into master Jun 1, 2023
@hoyon hoyon deleted the sc-27167/extend-build-pipeline-to-gather-stats-on branch June 1, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants