Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

1.2.5 angular-mocks copy change breaks tests using XML datatype #5429

Closed
matthughes opened this issue Dec 16, 2013 · 5 comments
Closed

1.2.5 angular-mocks copy change breaks tests using XML datatype #5429

matthughes opened this issue Dec 16, 2013 · 5 comments

Comments

@matthughes
Copy link

Apparently the XML datatype in Chrome/Firefox (and others?) contains pointers to itself. The change here (f69dc16) copies the HTTP response.

If you are mocking an HTTP service with XML datatype as the return type, copy() never terminates.

@matthughes
Copy link
Author

One solution would be to make angular.copy support recursive data structures. I made this change locally, but it is not enough.

If you just take this approach to copying an XMLDocument, you will be left with just the properties of the document, none of the methods that exist on the object's prototype. And then when someone tries to use one of those prototype methods, your test blows up.

So... we could add a special case for XMLDocument in copy and use cloneNode.

        } else if (source.cloneNode) {
          destination = source.cloneNode();
        }

But is that too specific to one type? Ideally users would be able to extend this copy method to support any type out there.

@matthughes
Copy link
Author

Here are my fixes I described above:

matthughes@205637e

@ghost ghost assigned btford Dec 30, 2013
@tbosch tbosch modified the milestones: 1.2.12, 1.2.11, 1.2.13 Feb 3, 2014
@btford btford modified the milestones: 1.2.14, 1.2.13 Feb 15, 2014
@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.1, 1.2.14 Mar 1, 2014
@btford btford modified the milestones: 1.3.0-beta.2, 1.3.0-beta.1 Mar 10, 2014
@tbosch tbosch modified the milestones: 1.3.0-beta.3, 1.3.0-beta.2 Mar 14, 2014
@matthughes
Copy link
Author

A workaround for this is to not use responseType at all in $http and manually parse the response as XML. For a lot of code this is probably the correct thing to do anyway as you should look at the response's ContentType to determine how to parse.

For example, you may think the response is XML, but a 500 error might return plaintext and your code will blow up.

@btford btford modified the milestones: 1.3.0-beta.4, 1.3.0-beta.3 Mar 24, 2014
@jeffbcross jeffbcross modified the milestones: 1.3.0-beta.6, 1.3.0-beta.5 Apr 3, 2014
@IgorMinar IgorMinar modified the milestones: 1.3.0-beta.7, 1.3.0-beta.6 Apr 22, 2014
@caitp caitp modified the milestones: 1.3.0-beta.9, 1.3.0-beta.8 May 9, 2014
@IgorMinar IgorMinar modified the milestones: 1.3.0, 1.3.0-beta.9 May 12, 2014
@btford btford removed the gh: issue label Aug 20, 2014
@petebacondarwin
Copy link
Member

angular.copy now handles cyclic structures (from 1.4.1 onwards - see 0e622f7) so I think we can close this.

@petebacondarwin
Copy link
Member

Oh but I just realised that there are two issues here. The cyclic one is fixed. The XML one is not.
I'll knock up a PR for the XML fix using @matthughes commit matthughes@205637e.

petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Sep 8, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Sep 8, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Sep 9, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Sep 9, 2015
petebacondarwin added a commit that referenced this issue Nov 19, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants