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

Add tests for local elements when elementFormDefault is set #46

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Toilal
Copy link
Contributor

@Toilal Toilal commented Feb 12, 2020

Related to goetas-webservices/xsd-reader#51

This currently adds a test in soap-client matching the issue. (and test is actually failing to demonstrate the issue)

@Toilal
Copy link
Contributor Author

Toilal commented Feb 12, 2020

The test is fixed with both goetas-webservices/xsd2php#108 and goetas-webservices/xsd-reader#52.

I have used https://github.com/gossi/composer-localdev-plugin to symlink to local development packages, works like a charm.

Please wait before merging those pull request, as I wish to add more test cases based on this ID3 Webservice, and try integration in my real world app.

@Toilal
Copy link
Contributor Author

Toilal commented Jun 19, 2020

Hi @goetas

I pretty sure you are really busy on many other subjects, but you could review and merge this pull requests and others related to this one ?

I'm currently running in an app that's now live in production the following patches from my pull requests.

        "patches": {
            "goetas-webservices/soap-client": {
                "Add ID3 wsdl test for local element property": "https://github.com/goetas-webservices/soap-client/pull/46.patch",
                "Add symfony 4+ support": "https://github.com/goetas-webservices/soap-client/pull/47.patch"
            },
            "goetas-webservices/xsd2php": {
                "Use elements local property to generate proper JMS mappings": "https://github.com/goetas-webservices/xsd2php/pull/108.patch"
            },
            "goetas-webservices/xsd-reader": {
                "Add local property to elements": "https://github.com/goetas-webservices/xsd-reader/pull/52.patch"
            }
        }

Everything is working properly with those patches, i'm pretty sure you could merge those pull requests and perform new release with confidence :)

Please let me know, best regards.

@goetas
Copy link
Member

goetas commented Jun 24, 2020

Hi, sorry if this is taking so long, but recently i've been really busy.

I'm really interested in goetas-webservices/xsd-reader#52 (and in the scenario that fixes) but I would like to have time to test it a bit better (I tend to follow the SOAP interpretation done by SoapUI... so would like to have some time to test it with it).

I think that the quality of both pull requests is very good, is just that i would like to find time to test them properly.

@goetas goetas self-assigned this Jun 24, 2020
@goetas
Copy link
Member

goetas commented Jun 24, 2020

same note for the tests as in goetas-webservices/xsd2php#108

@Toilal Toilal force-pushed the element-local-property branch 2 times, most recently from 7ca2a9d to e31a4d4 Compare August 22, 2020 00:29
@Toilal
Copy link
Contributor Author

Toilal commented Aug 22, 2020

Test have been simplified. They fail on CI because they need to run with goetas-webservices/xsd2php#108 and goetas-webservices/xsd-reader#52

@Toilal Toilal changed the title Add ID3 wsdl test for local element property Add tests for local elements when elementFormDefault is set Aug 22, 2020
@goetas
Copy link
Member

goetas commented Aug 22, 2020

Shouldnt we first merge goetas-webservices/xsd2php#108?

To make tests pass you can add @dev in the composer deps, in this way you will get the latest version

@goetas
Copy link
Member

goetas commented Aug 24, 2020

@Toilal
Copy link
Contributor Author

Toilal commented Aug 24, 2020

Thanks ! Thanks. I think we also need to release wsdl2php with this change in composer.json : goetas-webservices/wsdl2php#12

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.

2 participants