Skip to content

Conversation

@alexbenkey
Copy link
Contributor

added agnosticPrintString function to preexecutor, allowing for strings to be printed from the preexecutor to conform more with lit testing suite. Validation logic can now be handled externally in the test.h file as follows:

void assertEqual(double value, double expected, double epsilon, const char* msg)
{
if (value >= expected - epsilon && value <= expected + epsilon){
char buffer[256];
sprintf(buffer, "%s%g", msg, value);
agnosticPrintString(buffer);
}
}

optionally, the validation logic can be ignored entirely as the lit CHECK lines will fail if output is incorrect.

This solution simplifies the process of output generation from tests, while still allowing for descriptive messaging showing point of failure in a given test.

@alexbenkey
Copy link
Contributor Author

corresponding tests.h functions:

template<typename T>
size_t format_and_append_value(char * buffer, size_t valueLength, T value){

	if constexpr (std::is_same_v<T, double>){
		return sprintf(buffer + valueLength, "%.10f ", value);
	}
	else if constexpr(std::is_same_v<T, float>){
		return sprintf(buffer + valueLength, "%.10f ", (double)value);
	}
	if constexpr (std::is_same_v<T, const char *>){
		return sprintf(buffer + valueLength, "%s ", value);
	}
	else if constexpr(std::is_integral_v<T>){
		return sprintf(buffer + valueLength, "%lld ", (long long)value);
	}

	return (0);
}

template<typename... Args>
void __preexecute_print_case(const char *msg, Args... vals)
#ifdef PRE_EXECUTE_TEST
; // defined in preexecuter
#else
{
	char buffer[1024];
	size_t offset = 0;

	offset += sprintf(buffer, "%s", msg);
	((offset += format_and_append_value(buffer, offset, vals)), ...);

	cheerp::console_log(buffer);

}
#endif

return (void*)(void(*)())pre_execute_abs;
if (strcmp(funcName.c_str(), "assertEqualImpl") == 0)
return (void*)(void(*)())assertEqualImpl;
if (strcmp(funcName.c_str(), "agnosticPrintString") == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and we should remove this. __preexecute_print_case is just a superset of it.

@yuri91
Copy link
Member

yuri91 commented Dec 3, 2025

corresponding tests.h functions:

template<typename T>
size_t format_and_append_value(char * buffer, size_t valueLength, T value){

	if constexpr (std::is_same_v<T, double>){
		return sprintf(buffer + valueLength, "%.10f ", value);
	}
	else if constexpr(std::is_same_v<T, float>){
		return sprintf(buffer + valueLength, "%.10f ", (double)value);
	}
	if constexpr (std::is_same_v<T, const char *>){
		return sprintf(buffer + valueLength, "%s ", value);
	}
	else if constexpr(std::is_integral_v<T>){
		return sprintf(buffer + valueLength, "%lld ", (long long)value);
	}

	return (0);
}

template<typename... Args>
void __preexecute_print_case(const char *msg, Args... vals)
#ifdef PRE_EXECUTE_TEST
; // defined in preexecuter
#else
{
	char buffer[1024];
	size_t offset = 0;

	offset += sprintf(buffer, "%s", msg);
	((offset += format_and_append_value(buffer, offset, vals)), ...);

	cheerp::console_log(buffer);

}
#endif

we should not use sprintf, but instead use js-native methods to convert numbers to strings (like toFixed(10) and toString()), and log a client::String*

Copy link
Member

@yuri91 yuri91 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 now, but please squash all the commits together and put a relevant commit message

@yuri91 yuri91 merged commit 8907c03 into master Dec 8, 2025
1 check passed
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.

3 participants