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

feat: do not allow x:variable to override x:param #1330

Merged
merged 1 commit into from
Jan 5, 2021

Conversation

AirQuick
Copy link
Member

@AirQuick AirQuick commented Jan 4, 2021

With @run-as="external", only /x:description/x:param is supposed to take effect in SUT. x:variable is not.
However, x:variable can override x:param and effectively go into SUT. For example:

With this stylesheet

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet version="3.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">

	<xsl:param name="p" />
	<xsl:template name="get-p">
		<xsl:sequence select="$p" />
	</xsl:template>

	<xsl:param name="v" />
	<xsl:template name="get-v">
		<xsl:sequence select="$v" />
	</xsl:template>

</xsl:stylesheet>

and this XSpec

<?xml version="1.0" encoding="UTF-8"?>
<x:description run-as="external" stylesheet="test.xsl"
	xmlns:x="http://www.jenitennison.com/xslt/xspec">

	<x:param name="p">global-p</x:param>
	<x:variable name="v">global-v</x:variable>

	<x:scenario label="test">
		<x:variable name="p">local-p</x:variable>
		<x:variable name="v">local-v</x:variable>

		<x:scenario label="$p">
			<x:call template="get-p" />
			<x:expect label="test" />
		</x:scenario>

		<x:scenario label="$v">
			<x:call template="get-v" />
			<x:expect label="test" />
		</x:scenario>
	</x:scenario>

</x:description>
  • get-p returns local-p, not global-p, because x:variable overrides the value of $p.
  • get-v returns '', because x:variable by itself does not take effect in SUT.

This behavior could be considered as "by design". (In that sense, #991 is already partially possible. 😀)
But I think it's confusing. So this pull request prohibits x:variable from overriding /x:description/x:param. With this change, the example above is terminated with an error:

C:\xspec\test>..\bin\xspec.bat test.xspec
Creating Test Stylesheet...

ERROR: x:variable (named p) must not override x:param (named p)
...
*** Error compiling the test suite

Upon implementing #991, I'll extend this rule and prohibit x:variable from overriding //x:scenario/x:param as well.

@AirQuick AirQuick added this to the v2.1 milestone Jan 4, 2021
@AirQuick AirQuick requested a review from galtm January 4, 2021 18:48
Copy link
Member

@galtm galtm 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 agree that the override behavior would be confusing and it's better to disallow it.

@AirQuick AirQuick merged commit 0d35c8a into xspec:master Jan 5, 2021
@AirQuick AirQuick deleted the detect-variable-overriding-param branch January 5, 2021 04:50
@AirQuick AirQuick mentioned this pull request Feb 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants