search

umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.42k stars 2.67k forks source link

v8: Remove references to UmbracoTemplatePage in packaging unit tests #4953

Closed bjarnef closed 5 years ago

bjarnef commented 5 years ago

UmbracoTemplatePage has been removed in v8, but there are still 3 references to this in the Tests project. https://github.com/umbraco/Umbraco-CMS/search?utf8=✓&q=UmbracoTemplatePage&type=

I guess these three XML files can be removed since it is for legacy packages, which doesn't work on v8.

bjarnef commented 5 years ago

@nul800sebastiaan @Shazwazza can we remove these XML files?

Shazwazza commented 5 years ago

I don't think you can just remove these xml files, they look like they are being used in tests. If you want to modify the XML files to have the correct base page type in v8 then go for it, but you'll have to check for any failing tests and fixup accordingly.

bjarnef commented 5 years ago

@Shazwazza yes, but none if these support Umbraco v8 and I am unuse if they will. At least for Fanoe Starterkit, where we now have the newer default Starter Kit, so does it make sense to keep this in v8 branch?

Shazwazza commented 5 years ago

These "references" are just strings in xml package manifests which are used for unit tests. So sure, you can change these to be what v8 is like UmbracoViewPage but you'll need to make sure all tests continue to pass. These xml package manifests are purely just used for testing against.

bjarnef commented 5 years ago

But why do we need the xml package manifest for e.g. Fanoe starter kit when it is not supported in Umbraco v8 and we have the new default starter kit?

bjarnef commented 5 years ago

The repository is also archieved https://github.com/umbraco/Starterkits so I guess there are no plans to use or test this in v8, since we have the new starter kit.

Maybe I am missing something, but when do we need er need to run the unit test on Fanoe starter kit package in v8 branch?

Shazwazza commented 5 years ago

This has nothing to do with the actual fanoe starter kit, this has everything to do with simply testing package manifest files.

bjarnef commented 5 years ago

Okay, so it is just "dummy" data from package manifest xml based on Fanoe Starter kit? But won't it make sense to replace this and replace it with a package manifest xml based on the default starter kit instead?

Shazwazza commented 5 years ago

It's really not what data is in this xml file that's important, it's the tests that run against it to verify the packaging tools work. If you or someone wants to spend time to change these to be xml data that is based on v8 style packages then by all means please do but you'll most likely have to update unit tests that use these files to test against.

I've updated the title and added the up for grabs tag

nul800sebastiaan commented 5 years ago

This issue has been in the up for grabs state for quite a while now and it seems nobody is ready to pick this one up.

For now we'll close this issue to prevent the list of up for grabs from becoming very stale. We're happy to re-open it if someone still thinks it's good to work on this.

If anyone is about to pick this issue up to fix it, make sure to test first if you can reproduce the problem in the latest version before you start to work on it.

Thanks! Sebastiaan on behalf of Umbraco HQ.