Skip to content

Commit 5da8e18

Browse files
authored
Fix other.test_embind (#18866)
The test was not actually using the three compiler flags we tried to test there. Use getCompilerSetting which is a standard way to do this, that also asserts internally on the setting being properly defined.
1 parent 3442cda commit 5da8e18

File tree

3 files changed

+12
-19
lines changed

3 files changed

+12
-19
lines changed

test/embind/embind.test.js

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ module({
378378
});
379379

380380
BaseFixture.extend("string", function() {
381-
var stdStringIsUTF8 = (cm['EMBIND_STD_STRING_IS_UTF8'] === true);
381+
var stdStringIsUTF8 = (cm.getCompilerSetting('EMBIND_STD_STRING_IS_UTF8') === true);
382382

383383
test("non-ascii strings", function() {
384384

@@ -642,7 +642,7 @@ module({
642642
assert.throws(TypeError, function() { cm.const_ref_adder(0n, 1); });
643643
});
644644

645-
if (cm['ASSERTIONS']) {
645+
if (cm.getCompilerSetting('ASSERTIONS')) {
646646
test("can pass only number and boolean as floats with assertions", function() {
647647
assert.throws(TypeError, function() { cm.const_ref_adder(1, undefined); });
648648
assert.throws(TypeError, function() { cm.const_ref_adder(1, null); });
@@ -824,7 +824,7 @@ module({
824824
assert.equal("-2147483648", cm.long_to_string(-2147483648));
825825

826826
// passing out of range values should fail with assertions.
827-
if (cm['ASSERTIONS']) {
827+
if (cm.getCompilerSetting('ASSERTIONS')) {
828828
assert.throws(TypeError, function() { cm.char_to_string(-129); });
829829
assert.throws(TypeError, function() { cm.char_to_string(128); });
830830
assert.throws(TypeError, function() { cm.signed_char_to_string(-129); });
@@ -863,7 +863,7 @@ module({
863863
assert.equal(2147483648, cm.load_unsigned_long());
864864
});
865865

866-
if (cm['ASSERTIONS']) {
866+
if (cm.getCompilerSetting('ASSERTIONS')) {
867867
test("throws type error when attempting to coerce null to int", function() {
868868
var e = assert.throws(TypeError, function() {
869869
cm.int_to_string(null);
@@ -1044,13 +1044,13 @@ module({
10441044

10451045
assert.equal(undefined, vec.get(4));
10461046
// only test a negative index without assertions.
1047-
if (!cm['ASSERTIONS']) {
1047+
if (!cm.getCompilerSetting('ASSERTIONS')) {
10481048
assert.equal(undefined, vec.get(-1));
10491049
}
10501050
vec.delete();
10511051
});
10521052

1053-
if (cm['ASSERTIONS']) {
1053+
if (cm.getCompilerSetting('ASSERTIONS')) {
10541054
test("out of type range array index throws with assertions", function() {
10551055
var vec = cm.emval_test_return_vector();
10561056

@@ -1445,7 +1445,7 @@ module({
14451445
c.delete();
14461446
});
14471447

1448-
if (cm['ASSERTIONS']) {
1448+
if (cm.getCompilerSetting('ASSERTIONS')) {
14491449
test("assigning string or object to integer raises TypeError with assertions", function() {
14501450
var c = new cm.CustomStruct();
14511451
var e = assert.throws(TypeError, function() {
@@ -2509,7 +2509,8 @@ module({
25092509

25102510
BaseFixture.extend("function names", function() {
25112511
assert.equal('ValHolder', cm.ValHolder.name);
2512-
if (!cm['DYNAMIC_EXECUTION']) {
2512+
2513+
if (!cm.getCompilerSetting('DYNAMIC_EXECUTION')) {
25132514
assert.equal('', cm.ValHolder.prototype.setVal.name);
25142515
assert.equal('', cm.ValHolder.makeConst.name);
25152516
} else {

test/embind/helpers.js

Lines changed: 0 additions & 5 deletions
This file was deleted.

test/test_other.py

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,10 +2742,10 @@ def test_embind(self, extra_args):
27422742
(['-lembind', '-O1']),
27432743
(['-lembind', '-O2']),
27442744
(['-lembind', '-O2', '-sALLOW_MEMORY_GROWTH', test_file('embind/isMemoryGrowthEnabled=true.cpp')]),
2745+
(['-lembind', '-O2', '--closure=1']),
27452746
]
2747+
extra_args = extra_args + ['-sRETAIN_COMPILER_SETTINGS', '-sEXPORTED_RUNTIME_METHODS=getCompilerSetting']
27462748
test_cases = [t + extra_args for t in test_cases]
2747-
# closure compiler doesn't work with DYNAMIC_EXECUTION=0
2748-
test_cases.append((['-lembind', '-O2', '--closure=1']))
27492749
for args in test_cases:
27502750
print(args)
27512751
self.clear()
@@ -2760,13 +2760,10 @@ def test_embind(self, extra_args):
27602760
[EMXX, test_file('embind/embind_test.cpp'),
27612761
'--pre-js', test_file('embind/test.pre.js'),
27622762
'--post-js', test_file('embind/test.post.js'),
2763-
'--js-library', test_file('embind/helpers.js'),
27642763
'-sWASM_ASYNC_COMPILATION=0',
27652764
# This test uses a `CustomSmartPtr` class which has 1MB of data embedded in
27662765
# it which means we need more stack space than normal.
2767-
'-sSTACK_SIZE=2MB',
2768-
'-sDEFAULT_LIBRARY_FUNCS_TO_INCLUDE=$EMBIND_STD_STRING_IS_UTF8,$DYNAMIC_EXECUTION,$ASSERTIONS',
2769-
'-sEXPORTED_FUNCTIONS=EMBIND_STD_STRING_IS_UTF8,DYNAMIC_EXECUTION,ASSERTIONS'] + args)
2766+
'-sSTACK_SIZE=2MB'] + args)
27702767

27712768
if '-sDYNAMIC_EXECUTION=0' in args:
27722769
js_binary_str = read_file('a.out.js')

0 commit comments

Comments
 (0)