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.

Sign up for GitHub

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

Jump to bottom

Enable Test Get Module Logs #5793

Merged
merged 18 commits into from
Nov 4, 2021
Merged

Enable Test Get Module Logs #5793

merged 18 commits into from
Nov 4, 2021

Conversation

nimanch
Copy link
Contributor

@nimanch nimanch commented Nov 2, 2021 •
edited

In Certain instances, the date time generated by the Unit Test does not comply with RFC3339 Standard and hence the test fails with at a lower layer with error

For example DateTime : 2021-11-03T12:07:0Z returns
"Akka.Streams.Dsl.Framing+FramingException: Stream finished but there was a truncated final frame in the buffer"

This is because a request is sent to management socket with the datetime in incorrect format which in turn returns a Bad Request.

The fix is 2 fold

  1. Fix the unit test to conform to RFC 3339 Format
  2. Add checks in Managment API in Edge Agent to make sure the Since and Until Time conform to RFC3339 Standard
  3. Add Unit Test for the above mentioned case

Testing Done

  1. Ran E2E Test Locally
  2. Modified E2E Test to always use non-compliant DateTime ( Same as example above) and confirmed that test fails with same signature

Azure IoT Edge PR checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines and Best Practices

  • I have read the contribution guidelines.
  • Title of the pull request is clear and informative.
  • Description of the pull request includes a concise summary of the enhancement or bug fix.

Testing Guidelines

  • Pull request includes test coverage for the included changes.
  • Description of the pull request includes
    • concise summary of tests added/modified
    • local testing done.

Draft PRs

  • Open the PR in Draft mode if it is:
    • Work in progress or not intended to be merged.
    • Encountering multiple pipeline failures and working on fixes.

Note: We use the kodiakhq bot to merge PRs once the necessary checks and approvals are in place. When it merges a PR, kodiakhq converts the PR title to the commit title, PR description to the commit description, and squashes all the commits in the PR to a single commit. The net effect is that entire PR becomes a single commit. Please follow the best practices mentioned here for the PR title and description

nimanch and others added 9 commits October 7, 2021 04:21
@nimanch
Enable Rust Coverage (Azure#5616)
8c4f2e7 
This PR Re-enables Rust Coverage in Check in Pipelines
1. Remove the Dependency to on nightly build on Rust. Use the same version of rust as other pipelines
2. Use Cargo Tarpaulin for Create Code Coverage Reports. This was recommended by other teams within Microsoft developing rust code. (https://lib.rs/crates/cargo-tarpaulin)
3. Publish HTML Code Coverage Report Based on Coverage Report


Note : The Current Code Coverage stands at 44%.  I had to reduce the code coverage gate for Rust to 44%.  The intent is to increase the code coverage and then increase the gate.

Sample Build : https://iotedge.visualstudio.com/iotedge/_build/results?buildId=83180&view=codecoverage-tab
@nimanch
Increase Coverage to 47
32b7151
@nimanch
Merge branch 'Azure:release/1.2' into release/1.2
6ddbfdc
@nimanch
Merge branch 'Azure:release/1.2' into release/1.2
b82bd1e
@nimanch
Merge branch 'Azure:release/1.2' into release/1.2
5d7f815
@nimanch
Merge branch 'Azure:release/1.2' into release/1.2
3310042
@nimanch
Merge branch 'Azure:release/1.2' into release/1.2
223aaf1
@nimanch
Merge branch 'Azure:release/1.2' into release/1.2
32f7726
@nimanch
Enable Test Get Module Logs
6277e6a
damonbarry
damonbarry reviewed Nov 3, 2021
View reviewed changes
Copy link
Member

@damonbarry damonbarry left a comment

Choose a reason for hiding this comment

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

Looks good. I want Andrew to follow up on his comments, so I'll let him hit the "approve" button :)

@kodiakhq kodiakhq bot merged commit 2677657 into Azure:release/1.2 Nov 4, 2021
nimanch added a commit to nimanch/iotedge that referenced this pull request Nov 4, 2021
@nimanch
Enable Test Get Module Logs (Azure#5793)
1d834df 
In Certain instances, the date time generated by the Unit Test does not comply with RFC3339 Standard and hence the test fails with at a lower layer with error 

For example DateTime : 2021-11-03T12:07:0Z returns
"Akka.Streams.Dsl.Framing+FramingException: Stream finished but there was a truncated final frame in the buffer"

This is because a request is sent to management socket with the datetime in incorrect format which in turn returns a Bad Request.

The fix is 2 fold
1. Fix the unit test to conform to RFC 3339 Format
2. Add checks in Managment API in Edge Agent to make sure the Since and Until Time conform to RFC3339 Standard
3. Add Unit Test for the above mentioned case

Testing Done
1. Ran E2E Test Locally
2. Modified E2E Test to always use non-compliant DateTime ( Same as example above) and confirmed that test fails with same signature

## Azure IoT Edge PR checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

### General Guidelines and Best Practices
- [x] I have read the [contribution guidelines](https://github.com/azure/iotedge#contributing).
- [x] Title of the pull request is clear and informative.
- [x] Description of the pull request includes a concise summary of the enhancement or bug fix.

### Testing Guidelines
- [x] Pull request includes test coverage for the included changes.
- Description of the pull request includes 
	- [x] concise summary of tests added/modified
	- [x] local testing done.  

### Draft PRs
- Open the PR in `Draft` mode if it is:
	- Work in progress or not intended to be merged.
	- Encountering multiple pipeline failures and working on fixes.

_Note: We use the kodiakhq bot to merge PRs once the necessary checks and approvals are in place. When it merges a PR, kodiakhq converts the PR title to the commit title, PR description to the commit description, and squashes all the commits in the PR to a single commit. The net effect is that entire PR becomes a single commit. Please follow the best practices mentioned [here](https://chris.beams.io/posts/git-commit/#:~:text=The%20seven%20rules%20of%20a%20great%20Git%20commit,what%20and%20why%20vs.%20how%20For%20example%3A%20) for the PR title and description_
@nimanch nimanch mentioned this pull request Nov 4, 2021
Merged
6 tasks
nimanch added a commit to nimanch/iotedge that referenced this pull request Nov 4, 2021
@nimanch
Enable Test Get Module Logs (Azure#5793)
2a2012f 
In Certain instances, the date time generated by the Unit Test does not comply with RFC3339 Standard and hence the test fails with at a lower layer with error 

For example DateTime : 2021-11-03T12:07:0Z returns
"Akka.Streams.Dsl.Framing+FramingException: Stream finished but there was a truncated final frame in the buffer"

This is because a request is sent to management socket with the datetime in incorrect format which in turn returns a Bad Request.

The fix is 2 fold
1. Fix the unit test to conform to RFC 3339 Format
2. Add checks in Managment API in Edge Agent to make sure the Since and Until Time conform to RFC3339 Standard
3. Add Unit Test for the above mentioned case

Testing Done
1. Ran E2E Test Locally
2. Modified E2E Test to always use non-compliant DateTime ( Same as example above) and confirmed that test fails with same signature

## Azure IoT Edge PR checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

### General Guidelines and Best Practices
- [x] I have read the [contribution guidelines](https://github.com/azure/iotedge#contributing).
- [x] Title of the pull request is clear and informative.
- [x] Description of the pull request includes a concise summary of the enhancement or bug fix.

### Testing Guidelines
- [x] Pull request includes test coverage for the included changes.
- Description of the pull request includes 
	- [x] concise summary of tests added/modified
	- [x] local testing done.  

### Draft PRs
- Open the PR in `Draft` mode if it is:
	- Work in progress or not intended to be merged.
	- Encountering multiple pipeline failures and working on fixes.

_Note: We use the kodiakhq bot to merge PRs once the necessary checks and approvals are in place. When it merges a PR, kodiakhq converts the PR title to the commit title, PR description to the commit description, and squashes all the commits in the PR to a single commit. The net effect is that entire PR becomes a single commit. Please follow the best practices mentioned [here](https://chris.beams.io/posts/git-commit/#:~:text=The%20seven%20rules%20of%20a%20great%20Git%20commit,what%20and%20why%20vs.%20how%20For%20example%3A%20) for the PR title and description_
This was referenced Nov 4, 2021
Merged
Merged
kodiakhq bot pushed a commit that referenced this pull request Nov 8, 2021
@nimanch
Enable Get Module Logs for Proxy Environment. (#5811)
2c52929 
The fix was made in #5793. Missed Enabling it in Proxy Environment
E2E Pipeline run :https://dev.azure.com/msazure/One/_build/results?buildId=48715036&view=results
## Azure IoT Edge PR checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

### General Guidelines and Best Practices
- [x] I have read the [contribution guidelines](https://github.com/azure/iotedge#contributing).
- [x] Title of the pull request is clear and informative.
- [x] Description of the pull request includes a concise summary of the enhancement or bug fix.

### Testing Guidelines
- [x] Pull request includes test coverage for the included changes.
- Description of the pull request includes 
	- [x] concise summary of tests added/modified
	- [x] local testing done.  

### Draft PRs
- Open the PR in `Draft` mode if it is:
	- Work in progress or not intended to be merged.
	- Encountering multiple pipeline failures and working on fixes.

_Note: We use the kodiakhq bot to merge PRs once the necessary checks and approvals are in place. When it merges a PR, kodiakhq converts the PR title to the commit title, PR description to the commit description, and squashes all the commits in the PR to a single commit. The net effect is that entire PR becomes a single commit. Please follow the best practices mentioned [here](https://chris.beams.io/posts/git-commit/#:~:text=The%20seven%20rules%20of%20a%20great%20Git%20commit,what%20and%20why%20vs.%20how%20For%20example%3A%20) for the PR title and description_
nimanch added a commit to nimanch/iotedge that referenced this pull request Nov 8, 2021
@nimanch
Enable Get Module Logs for Proxy Environment. (Azure#5811)
2807e33 
The fix was made in Azure#5793. Missed Enabling it in Proxy Environment
E2E Pipeline run :https://dev.azure.com/msazure/One/_build/results?buildId=48715036&view=results

This checklist is used to make sure that common guidelines for a pull request are followed.
- [x] I have read the [contribution guidelines](https://github.com/azure/iotedge#contributing).
- [x] Title of the pull request is clear and informative.
- [x] Description of the pull request includes a concise summary of the enhancement or bug fix.
- [x] Pull request includes test coverage for the included changes.
- Description of the pull request includes
	- [x] concise summary of tests added/modified
	- [x] local testing done.
- Open the PR in `Draft` mode if it is:
	- Work in progress or not intended to be merged.
	- Encountering multiple pipeline failures and working on fixes.

_Note: We use the kodiakhq bot to merge PRs once the necessary checks and approvals are in place. When it merges a PR, kodiakhq converts the PR title to the commit title, PR description to the commit description, and squashes all the commits in the PR to a single commit. The net effect is that entire PR becomes a single commit. Please follow the best practices mentioned [here](https://chris.beams.io/posts/git-commit/#:~:text=The%20seven%20rules%20of%20a%20great%20Git%20commit,what%20and%20why%20vs.%20how%20For%20example%3A%20) for the PR title and description_
kodiakhq bot pushed a commit that referenced this pull request Nov 8, 2021
@nimanch
[Cherry-Pick] Enable Test Get Module Logs (#5801)
7c20254 
This is a straight cherry pick of #5793 

## Azure IoT Edge PR checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

### General Guidelines and Best Practices
- [x] I have read the [contribution guidelines](https://github.com/azure/iotedge#contributing).
- [x] Title of the pull request is clear and informative.
- [x] Description of the pull request includes a concise summary of the enhancement or bug fix.

### Testing Guidelines
- [x] Pull request includes test coverage for the included changes.
- Description of the pull request includes 
	- [x] concise summary of tests added/modified
	- [x] local testing done.  

### Draft PRs
- Open the PR in `Draft` mode if it is:
	- Work in progress or not intended to be merged.
	- Encountering multiple pipeline failures and working on fixes.

_Note: We use the kodiakhq bot to merge PRs once the necessary checks and approvals are in place. When it merges a PR, kodiakhq converts the PR title to the commit title, PR description to the commit description, and squashes all the commits in the PR to a single commit. The net effect is that entire PR becomes a single commit. Please follow the best practices mentioned [here](https://chris.beams.io/posts/git-commit/#:~:text=The%20seven%20rules%20of%20a%20great%20Git%20commit,what%20and%20why%20vs.%20how%20For%20example%3A%20) for the PR title and description_
kodiakhq bot pushed a commit that referenced this pull request Nov 9, 2021
@nimanch
[Cherry-Pick]Enable Test Get Module Logs (#5800)
3c3efa8 
This is a cherry-pick for #5793 and #5811

## Azure IoT Edge PR checklist:
nimanch added a commit to nimanch/iotedge that referenced this pull request Nov 12, 2021
@nimanch
Revert Changes by PR Azure#5793, Time Stamp handling needs to be hand…
e8d01d3 
…led in the edgelet code. BThis change broke the Portal Log Viewing since since and until time can take unix as well as esimple language time
@nimanch nimanch mentioned this pull request Nov 12, 2021
Merged
6 tasks
kodiakhq bot pushed a commit that referenced this pull request Nov 12, 2021
@nimanch
Revert Changes to Handle Validation of Since and Until Time (#5840)
f7f4b89 
This change was made in #5793, but broke the Portal Log Viewing. We need to handle the validation logic in edgelet code

## Azure IoT Edge PR checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

### General Guidelines and Best Practices
- [x] I have read the [contribution guidelines](https://github.com/azure/iotedge#contributing).
- [x] Title of the pull request is clear and informative.
- [x] Description of the pull request includes a concise summary of the enhancement or bug fix.

### Testing Guidelines
- [x] Pull request includes test coverage for the included changes.
- Description of the pull request includes 
	- [x] concise summary of tests added/modified
	- [x] local testing done.  

### Draft PRs
- Open the PR in `Draft` mode if it is:
	- Work in progress or not intended to be merged.
	- Encountering multiple pipeline failures and working on fixes.

_Note: We use the kodiakhq bot to merge PRs once the necessary checks and approvals are in place. When it merges a PR, kodiakhq converts the PR title to the commit title, PR description to the commit description, and squashes all the commits in the PR to a single commit. The net effect is that entire PR becomes a single commit. Please follow the best practices mentioned [here](https://chris.beams.io/posts/git-commit/#:~:text=The%20seven%20rules%20of%20a%20great%20Git%20commit,what%20and%20why%20vs.%20how%20For%20example%3A%20) for the PR title and description_
damonbarry added a commit to damonbarry/iotedge that referenced this pull request Apr 15, 2022
@damonbarry
[Cherry-Pick]Enable Test Get Module Logs (Azure#5800)
0a47374 
This is a cherry-pick for Azure#5793 and Azure#5811
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@damonbarry damonbarry damonbarry left review comments

@and-rewsmith and-rewsmith and-rewsmith approved these changes

Assignees
No one assigned
Labels
ready-to-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nimanch @damonbarry @and-rewsmith