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

[Bug]: Fail builds on types of cargo failures #575

Closed
1 task done
makubacki opened this issue Sep 22, 2023 · 1 comment · Fixed by #578
Closed
1 task done

[Bug]: Fail builds on types of cargo failures #575

makubacki opened this issue Sep 22, 2023 · 1 comment · Fixed by #578
Assignees
Labels
state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:medium Important with a moderate impact

Comments

@makubacki
Copy link
Member

Is there an existing issue for this?

  • I have searched existing issues

Current Behavior

We recently encountered an issue where cargo make errors did not get noticed for a while because the build continued.

For example:

  [cargo-make] Execute Command: "cargo" "tarpaulin" "-p" \
    "HelloWorldRustDxe,RustBootServicesAllocatorDxe"

  cargo_tarpaulin::config: Creating config
  cargo_tarpaulin: Running Tarpaulin
  cargo_tarpaulin: Building project
  cargo_tarpaulin::cargo: Cleaning project
  error: invalid character `,` in pkgid:
    `HelloWorldRustDxe,RustBootServicesAllocatorDxe`, characters must
    be Unicode XID characters (numbers, `-`, `_`, or most letters)
  cargo_tarpaulin: Cargo failed to run! Error: cargo run failed
  Error: "Cargo failed to run! Error: cargo run failed"
  [cargo-make] ERROR - Error while executing command, exit code: 1
  [cargo-make] WARN - Build Failed.

Expected Behavior

It would be helpful for the build to fail on errors with clear references to errors.

Steps To Reproduce

Review the MsCorePkg pipeline output as of commit dec8f57d3a5dc3f270f6b18940e99ba114d25f7a.

Build Environment

- Mu Plus at commit dec8f57d3a5dc3f270f6b18940e99ba114d25f7a
- Build agent details
  - https://github.com/actions/runner-images/blob/win22/20230918.1/images/win/Windows2022-Readme.md
  - https://github.com/actions/runner-images/releases/tag/win22%2F20230918.1

Version Information

- Rust 1.71.1
- Cargo-make 0.37.1
- Cargo-tarpaulin 0.27.0

Urgency

Medium

Are you going to fix this?

Someone else needs to fix it

Do you need maintainer feedback?

No maintainer feedback needed

Anything else?

No response

@makubacki makubacki added state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working labels Sep 22, 2023
@github-actions github-actions bot added urgency:medium Important with a moderate impact state:needs-owner Needs an issue owner to be assigned and removed state:needs-owner Needs an issue owner to be assigned labels Sep 22, 2023
@Javagedes
Copy link
Contributor

This is in regard to stuart_ci_build plugins. Specifically RustHostUnitTestPlugin that does not fail when the command itself fails due to an error unrelated to individual test failures.

Javagedes added a commit that referenced this issue Sep 27, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Oct 17, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Oct 17, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Oct 18, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Oct 18, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Oct 19, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Oct 20, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Oct 23, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Dec 16, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Dec 18, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
kenlautner pushed a commit that referenced this issue Dec 18, 2023
## Description

Makes multiple improvements to the RustHostUnitTestPlugin to support a
more reliable experience:

1. The plugin now fails when a test fails to compile or cargo tarpaulin
fails to run.
2. The plugin now logs the reason for failing as a warning in addition
to logging the output to the xml file.
3. The plugin now properly filters results to only include results of
rust crates inside the package being tested.

Closes #575 

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [ ] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [ ] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

1. Verified only rust crates in the package being tested, are tested
2. Verified coverage results from rust crates being tested only show
coverage values of rust crates in that specific EDKII package (If a
crate has a dependency on another crate in a different EDKII package,
the coverage results for the other crate would show up.
3. Verified we properly log (and fail the plugin) when a test fails to
compile
4. Verified we properly log (and fail the plugin) when the command as a
whole fails for some reason
5. Verified we log (in addition to the xml log) the particular test that
is failing the plugin

## Integration Instructions

N/A
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:needs-triage Needs to triaged to determine next steps type:bug Something isn't working urgency:medium Important with a moderate impact
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants