Add practical node tests (lodash...) #5
Conversation
|
This seems broken on my system, the image doesn't seem to have git. |
|
Retriggered testing, as the containers have changed now |
|
Rebased with latest poetry merges. |
dcermak
force-pushed
the
test-lodash
branch
5 times, most recently
from
069668f
to
4e78da2
Compare
dcermak
force-pushed
the
test-lodash
branch
3 times, most recently
from
28c568f
to
4c8a8ec
Compare
|
@evrardjp Would you take a look at what I've done to your node tests (unfortunately I cannot request a review from you, as you submitted the PR)? The CI should be passing now 🤞. |
dcermak
force-pushed
the
test-lodash
branch
2 times, most recently
from
e50dbf6
to
12eb242
Compare
test_node.py
Outdated
| NpmPackageTest( | ||
| repository_url="https://github.com/expressjs/express.git", | ||
| build_command="""npm config set shrinkwrap false && | ||
| npm rm --silent --save-dev connect-redis && |
There was a problem hiding this comment.
Ok the only thing I find to say is that those lines are ugly, and would be better if they were all indented 4 spaces below build_command.
test_node.py
Outdated
| repository_url="https://github.com/jprichardson/node-fs-extra", | ||
| build_command="npm install && npm run unit", | ||
| ), | ||
| pytest.mark.xfail( |
There was a problem hiding this comment.
nit: I can't shake why I would do it, but I would pass the marks as kwargs of NpmPackageTest, and just do a single list comprehension for all the tests [ pkg.to_pytest_param(marks=self.marks) for pkg in (NpmPackageTest(build_command=, repository_url=, marks= marks being only set on L103+ . WDYT? Will it be more readable for non pythonistas?
There was a problem hiding this comment.
Yeah, originally I wanted to treat NpmPackageTest as a pure data container, but now that it also contains some "business logic", I think adding the mark there makes sense as well.
|
For some reason the react.js tests are failing on github actions but not locally for me (at least not when I run them serially). I guess that the react tests are too fat and fail if you overload the host. So either we don't run them in parallel, or we just drop them from the test matrix. I'm actually more inclined to dropping react, as it will make the test matrix really huge. |
|
Well, github actions isn't using I am fine with dropping react, but we'll have to think about those cases in the future. |
|
I've opted to skip the react tests for now until we figure out how we can run certain tests serially and others in parallel. |
No description provided.