Skip to content

Conversation

@wesleybl
Copy link
Member

We can use, for example:

make test package=plone.namedfile

or

make test package=plone.namedfile test=test_scaling

We can use, for example:

`make test package=plone.namedfile`

or

`make test package=plone.namedfile test=test_scaling`
Comment on lines +127 to +134
Use the `make test` target to run unit tests for a package:

```bash
# Run all unit tests for a package (package is required)
make test package=plone.namedfile

# Run a specific test
make test package=plone.namedfile test=test_scaling
Copy link
Contributor

Choose a reason for hiding this comment

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

Use generic shell syntax highlighter, don't use comments for instructions because they are low contrast in the rendered code block, and make it easier to copy-paste a single command.

Suggested change
Use the `make test` target to run unit tests for a package:
```bash
# Run all unit tests for a package (package is required)
make test package=plone.namedfile
# Run a specific test
make test package=plone.namedfile test=test_scaling
Use the `make test` target to run unit tests for a package.
For example, run all unit tests for a given package.
```shell
make test package=plone.namedfile
```
Run a specific test in the given package.
```shell
make test package=plone.namedfile test=test_scaling

Copy link
Member Author

Choose a reason for hiding this comment

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

@stevepiercy We need to specify somewhere that the package parameter is required.

Copy link
Contributor

Choose a reason for hiding this comment

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

@wesleybl that's what line 135 does.

Copy link
Member Author

Choose a reason for hiding this comment

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

Line 135 describes how to run only one test from a given package.

What I'm trying to say is that it's not possible to execute the command:

make test

it's necessary to pass the package:

make test package=plone.namedfile

See: https://github.com/plone/buildout.coredev/pull/1059/files#diff-35b89ef65700ec4da068cc82250d658677a5aaabec8b91e21d0d3e69987c949fR130

Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't understand what you mean. Line 135 mentions "the given package", and is followed by line 138, which has the package parameter.

Copy link
Member

Choose a reason for hiding this comment

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

@wesleybl But the run-tests.sh script says it is supposed to be run using make test, and that made me realize you are adding a new test target even though there is already one: https://github.com/plone/buildout.coredev/blob/6.2/Makefile#L468

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, in version 6.1 it doesn't exist: https://github.com/plone/buildout.coredev/blob/6.1/Makefile

Copy link
Member

Choose a reason for hiding this comment

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

I see, it was added in #1020

It would be nice to have it for both 6.1 and 6.2, with the same implementation in both, and improve it to have a way to optionally run the tests for only one package.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems the target already accepts individual packages and tests:

https://github.com/plone/buildout.coredev/blob/6.2/Makefile#L468

I'll test it later.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked and actually the target in branch 6.2 doesn't support running only the tests from a specific package.

Comment on lines +138 to +140
- Packages in development with modern layout (`src/package/src/`)
- Packages in development with legacy layout (`src/package/`)
- Packages installed in the virtual environment
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to capitalize a list item when it is not a sentence.

Suggested change
- Packages in development with modern layout (`src/package/src/`)
- Packages in development with legacy layout (`src/package/`)
- Packages installed in the virtual environment
- packages in development with modern layout (`src/package/src/`)
- packages in development with legacy layout (`src/package/`)
- packages installed in the virtual environment


You can also run tests directly with `zope-testrunner`:

```bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
```bash
```shell

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

This looks helpful for the use case of testing one package.

The main purpose of buildout.coredev is to provide a place to test all packages together. So it would be nice to also make the package parameter optional, and in that case run the tests from all packages. Then we could update the Jenkins builds to use this instead of the buildout.

@wesleybl
Copy link
Member Author

The main purpose of buildout.coredev is to provide a place to test all packages together. So it would be nice to also make the package parameter optional, and in that case run the tests from all packages

@davisagli The problem is that site-packages doesn't only contain Plone packages. Therefore, I can't pass the site-packages folder to the --test-path of zope-testrunner. We would need a list of the packages to be tested, or filter by a prefix.

@wesleybl
Copy link
Member Author

@davisagli I see we have this list of packages to be tested for the buildout:

https://github.com/plone/buildout.coredev/blob/6.2/tests.cfg#L6

Can we read from there?

@davisagli
Copy link
Member

@wesleybl Oh, I see we already have this script to run all the tests (with the same list): https://github.com/plone/buildout.coredev/blob/6.2/run-tests.sh

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.

4 participants