8000 xml module: Better change detection, improved tests by dagwieers · Pull Request #28460 · ansible/ansible · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

xml module: Better change detection, improved tests #28460

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
Merged
merged 1 commit into from
Aug 25, 2017
Merged

xml module: Better change detection, improved tests #28460

merged 1 commit into from
Aug 25, 2017

Conversation

dagwieers
Copy link
Contributor
@dagwieers dagwieers commented Aug 21, 2017 •
edited
Loading
SUMMARY

This PR includes:

  • Improvements to change-detection by comparing 2 objectified XML trees
  • Implement better integration tests by comparing 2 files using copy
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

xml

ANSIBLE VERSION

v2.4

@ansibot
Copy link
Contributor
ansibot commented Aug 21, 2017

cc @cmprescott @sm4rk0 @tbielawa
click here for bot help

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bugfix_pull_request community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community. labels Aug 21, 2017
@dagwieers
Copy link
Contributor Author

@richardcs I modified the integration tests to use an assert to compare content, rather than calling diff. This should be portable (but is slightly harder to troubleshoot as a diff is not shown). I also implemented a diff module that can compare files, however you would need an equivalent win_diff. for testing win_xml.

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 21, 2017
@dagwieers dagwieers force-pushed the xml-changed branch 2 times, most recently from 14f16b4 to 6df1b48 Compare August 21, 2017 01:57
@ansible ansible deleted a comment from ansibot Aug 21, 2017
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Aug 21, 2017
@dagwieers dagwieers force-pushed the xml-changed branch 2 times, most recently from 2b3bd8e to 7317dec Compare August 21, 2017 02:22
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 21, 2017
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Aug 21, 2017
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 24, 2017
@dagwieers dagwieers force-pushed the xml-changed branch 2 times, most recently from f4fd598 to 0bfd2e6 Compare August 24, 2017 23:35
@dagwieers dagwieers changed the title xml: Don't consider modifying string as a change xml: Better detection of changes, integration tests improved Aug 24, 2017
@dagwieers dagwieers changed the title xml: Better detection of changes, integration tests improved xml module: Better change detection, integration tests improved Aug 24, 2017
@dagwieers dagwieers changed the title xml module: Better change detection, integration tests improved xml module: Better change detection, improved tests Aug 24, 2017
@ansible ansible deleted a comment from ansibot Aug 24, 2017
@dagwieers
Copy link
Contributor Author

@richardcs This is the final set of integration tests. Now using only copy, xml and assert, but using diff output instead. This is much more convenient than before and since win_copy already has diff support, you only need to add diff-support to win_xml

@dagwieers
xml module: Better change detection, improved tests
afd45ea 
This PR includes:

- Improvements to change-detection by comparing 2 objectified XML trees
- Implement better integration tests by comparing 2 files using copy
@dagwieers dagwieers merged commit 2634ef9 into ansible:devel Aug 25, 2017
@dagwieers dagwieers added the m:xml This issue/PR relates to the xml module. label Sep 27, 2017
prasadkatti pushed a commit to prasadkatti/ansible that referenced this pull request Oct 1, 2017
@dagwieers
xml module: Better change detection, improved tests (ansible#28460)
f7ee1cb 
This PR includes:

- Improvements to change-detection by comparing 2 objectified XML trees
- Implement better integration tests by comparing 2 files using copy
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. m:xml This issue/PR relates to the xml module. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dagwieers @ansibot @bcoca
0