Skip to content

test: fix apitest, add new unit tests and benchmarks, normalize output#1467

Merged
KernelDeimos merged 1 commit intoHeyPuter:mainfrom
XiaochenCui:test/apitest
Aug 27, 2025
Merged

test: fix apitest, add new unit tests and benchmarks, normalize output#1467
KernelDeimos merged 1 commit intoHeyPuter:mainfrom
XiaochenCui:test/apitest

Conversation

@XiaochenCui
Copy link
Copy Markdown
Contributor

this PR updates the apitest:

  • make all tests works
  • add detailed docs for usage and design
  • add new unit tests and benchmark
  • add special output to work with the CI script

Copy link
Copy Markdown
Contributor

@KernelDeimos KernelDeimos 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, only a couple documentation-related comments - will merge once those are addressed.

' --config=<path> (required) Path to configuration file\n' +
' --report=<path> (optional) Output file for full test results\n' +
' --suite=<name> (optional) Run only tests with matching suite name\n' +
' --stop-on-failure (optional) Stop execution on first test failure\n' +
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missing --onlycase here - what does it do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

--onlycase doesn't work, I propose to remove this arg.

A bigger problem is that the current implementation doesn't support run a specific case - since case can be nested and we don't know a case's children before running it. e.g:

        // we cannot skip 'test_a' when filter=test_b, since test_b might be a child of 'test_a'
        await t.case('test_a', async () => {
            
            await t.case('test_b', async () => {
                ...
            });

a good news is --suite now supported:

  • Filter tests by suite name:

    node ./tools/api-tester/apitest.js --config=./tools/api-tester/config.yml --unit --suite=mkdir
  • Filter benchmarks by name:

    node ./tools/api-tester/apitest.js --config=./tools/api-tester/config.yml --bench --suite=stat_intensive_1

// TODO (xiaochen): `t.mkdir('a/b/c')` throws 409/422, unify the
// behavior of these two cases.
expect(e.response.status).equal(422);
expect(e.response.status).oneOf([409, 422]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this error code dependent on the filesystem implementation? (whether it's memoryfs or puterfs)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The error code depends on the state left by previous tests. I left a TODO here to (plan to) make it more deterministic and meaningful.

@KernelDeimos KernelDeimos merged commit c34bd81 into HeyPuter:main Aug 27, 2025
4 checks 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.

2 participants