Skip to content

Use simple response file format in test code. NFC #22976

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.

Sign up for GitHub

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

Merged
sbc100 merged 1 commit into from
Nov 21, 2024
Merged

Use simple response file format in test code. NFC #22976

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions test/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7155,7 +7155,7 @@ def test_exported_response(self):
return 0;
}
'''
create_file('exps', '["_main","_other_function"]')
create_file('exps', '_main\n_other_function\n')

self.set_setting('EXPORTED_FUNCTIONS', '@exps')
self.do_run(src, '''waka 5!''')
Expand All @@ -7170,12 +7170,12 @@ def test_large_exported_response(self):
extern "C" {
'''

js_funcs = []
rsp_file_lines = []
num_exports = 5000
count = 0
while count < num_exports:
src += 'int exported_func_from_response_file_%d () { return %d;}\n' % (count, count)
js_funcs.append('_exported_func_from_response_file_%d' % count)
rsp_file_lines.append('_exported_func_from_response_file_%d' % count)
count += 1

src += r'''
Expand All @@ -7191,8 +7191,8 @@ def test_large_exported_response(self):
}
'''

js_funcs.append('_main')
create_file('large_exported_response.json', json.dumps(js_funcs))
rsp_file_lines.append('_main')
create_file('large_exported_response.json', '\n'.join(rsp_file_lines) + '\n')

self.set_setting('EXPORTED_FUNCTIONS', '@large_exported_response.json')
self.do_run(src, 'waka 4999!')
Expand Down Expand Up @@ -8193,8 +8193,8 @@ def test_asyncify_unused(self):
'onlylist_b': (['-sASYNCIFY_ONLY=["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()"]'], True),
'onlylist_c': (['-sASYNCIFY_ONLY=["main","__original_main","foo(int, double)","baz()","c_baz"]'], False),
'onlylist_d': (['-sASYNCIFY_ONLY=["foo(int, double)","baz()","c_baz","Structy::funcy()"]'], False),
'onlylist_b_response': ([], True, '["main","__original_main","foo(int, double)","baz()","c_baz","Structy::funcy()"]'),
'onlylist_c_response': ([], False, '["main","__original_main","foo(int, double)","baz()","c_baz"]'),
'onlylist_b_response': ([], True, 'main\n__original_main\nfoo(int, double)\nbaz()\nc_baz\nStructy::funcy()\n'),
'onlylist_c_response': ([], False, 'main\n__original_main\nfoo(int, double)\nbaz()\nc_baz\n'),
Copy link
Member

@kripken kripken Nov 21, 2024

Choose a reason for hiding this comment

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

In this case, isn't the original clearer actually? It avoids \nc and such, where the reader needs to think how to separate into tokens.

And isn't it good to keep testing for the older format, or do we have that testing elsewhere?

Copy link
Collaborator Author

@sbc100 sbc100 Nov 21, 2024

Choose a reason for hiding this comment

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

See the PR description (I updated it). We have plenty of testing for old/new format.

Copy link
Member

@kripken kripken Nov 21, 2024

Choose a reason for hiding this comment

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

Sorry I missed that. Then I still think this would be clearer the other way, but I don't feel strongly.

Copy link
Collaborator Author

@sbc100 sbc100 Nov 21, 2024

Choose a reason for hiding this comment

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

In this case, isn't the original clearer actually? It avoids \nc and such, where the reader needs to think how to separate into tokens.

Its easier to read here in the python, but if you look at the test directory after the test runs and inspect the resp file, the one-element-per-line format is way easier to read.

I'd like to one day deprecate the old rsp json format.. (so not using outside of test_dash_s_list_parsing is a good first step)

Copy link
Collaborator Author

@sbc100 sbc100 Nov 21, 2024

Choose a reason for hiding this comment

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

Its a little complex here because this python test file is trying to to encode the contetns of a file into a single-line-string in python, which will always look at little odd.

})
def test_asyncify_lists(self, args, should_pass, response=None):
if response is not None:
Expand Down
Loading