Skip to content

Skip body buffer autest if plugin was not built#9761

Merged
duke8253 merged 1 commit intoapache:masterfrom
duke8253:master-skip_test
Jun 2, 2023
Merged

Skip body buffer autest if plugin was not built#9761
duke8253 merged 1 commit intoapache:masterfrom
duke8253:master-skip_test

Conversation

@duke8253
Copy link
Copy Markdown
Contributor

No description provided.

@duke8253 duke8253 added this to the 10.0.0 milestone May 31, 2023
@duke8253 duke8253 self-assigned this May 31, 2023
maskit
maskit previously approved these changes May 31, 2023
Copy link
Copy Markdown
Member

@maskit maskit left a comment

Choose a reason for hiding this comment

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

Makes sense, but we may want to always build the plugin and run this test like we do for hook_add test because it looks like this test checks TS API functionality but not the plugin itself.

@duke8253
Copy link
Copy Markdown
Contributor Author

Makes sense, but we may want to always build the plugin and run this test like we do for hook_add test because it looks like this test checks TS API functionality but not the plugin itself.

Well, this test fails if I build without --enable-example-plugins, so I think we probably should add this skip.

@maskit
Copy link
Copy Markdown
Member

maskit commented May 31, 2023

Well, this test fails if I build without --enable-example-plugins

I understand it. So the change makes sense in that sense

, so I think we probably should add this skip.

If we just want to avoid autest test failure, yes.

I'm saying we should always build the plugin (or a copy of the plugin) to see if TS API works even if ATS is built without --enable-example-plugins.

@duke8253
Copy link
Copy Markdown
Contributor Author

duke8253 commented Jun 1, 2023

Well, this test fails if I build without --enable-example-plugins

I understand it. So the change makes sense in that sense

, so I think we probably should add this skip.

If we just want to avoid autest test failure, yes.

I'm saying we should always build the plugin (or a copy of the plugin) to see if TS API works even if ATS is built without --enable-example-plugins.

Gotcha, then maybe we should put a copy of the request_buffer.c in the tests/tools/plugins/ folder so it gets built when we run the test.

@bneradt
Copy link
Copy Markdown
Contributor

bneradt commented Jun 1, 2023

I understand @maskit 's comment, but I wouldn't want the stop testing the example plugin either. I suppose ideally we would keep the example plugin test as is and add a second test that tests the API. But adding a direct copy doesn't seem to add too much value. Since our CI jobs build and run autests with --enable-example-plugins I think we're fine just adding the skip for now, personally speaking.

@maskit
Copy link
Copy Markdown
Member

maskit commented Jun 1, 2023

Ok, then adding the skip is progress for the ideal state in terms of code, although we'll lost a guarantee that the API is tested (someone might remove --enable-example-plugins from CI build in the future).

@duke8253 duke8253 merged commit 809b800 into apache:master Jun 2, 2023
@duke8253
Copy link
Copy Markdown
Contributor Author

duke8253 commented Jun 2, 2023

I'll find some time to make a new test to testing that API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants