Skip to content
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

Jump to bottom

Simplify JsThemis packaging and test launch #618

Merged
merged 5 commits into from
Apr 8, 2020
Merged

Simplify JsThemis packaging and test launch #618

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a94725c
Improve JsThemis packaging
ilammy Apr 2, 2020
9ffa7ce
Move and simplify JsThemis installation targets
ilammy Apr 3, 2020
d534906
Simplify JsThemis unit test launch
ilammy Apr 3, 2020
7d588f8
Convenience "jsthemis" target
ilammy Apr 6, 2020
516de7c
Move test directory into JsThemis source tree
ilammy Apr 6, 2020
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
4 changes: 0 additions & 4 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,6 @@ jobs:
echo "node --version: $(node --version)"
echo "npm --version: $(npm --version)"
# Run JsThemis tests
rm -rf src/wrappers/themis/jsthemis/node_modules
make test_js
- run:
name: Test with Node.js v8 (maintenance)
Expand All @@ -514,7 +513,6 @@ jobs:
echo "node --version: $(node --version)"
echo "npm --version: $(npm --version)"
# Run JsThemis tests
rm -rf src/wrappers/themis/jsthemis/node_modules
make test_js
# Run WasmThemis tests
make BUILD_PATH=build-wasm test_wasm
Expand All @@ -527,7 +525,6 @@ jobs:
echo "node --version: $(node --version)"
echo "npm --version: $(npm --version)"
# Run JsThemis tests
rm -rf src/wrappers/themis/jsthemis/node_modules
make test_js
# Run WasmThemis tests
make BUILD_PATH=build-wasm test_wasm
Expand All @@ -540,7 +537,6 @@ jobs:
echo "node --version: $(node --version)"
echo "npm --version: $(npm --version)"
# Run JsThemis tests
rm -rf src/wrappers/themis/jsthemis/node_modules
make test_js
# Run WasmThemis tests
make BUILD_PATH=build-wasm test_wasm
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ _Code:_
- **Node.js**

- New class `SymmetricKey` can be used to generate symmetric keys for Secure Cell ([#562](https://github.com/cossacklabs/themis/pull/562)).
- New makefile target `make jsthemis` can be used to build JsThemis from source ([#618](https://github.com/cossacklabs/themis/pull/618)).

- **PHP**

Expand Down Expand Up @@ -369,6 +370,8 @@ _Infrastructure:_
- Automated benchmarking harness is now tracking Themis performance. See [`benches`](https://github.com/cossacklabs/themis/tree/master/benches/) ([#580](https://github.com/cossacklabs/themis/pull/580)).
- Added automated tests for all code samples in documentation, ensuring they are always up-to-date ([#600](https://github.com/cossacklabs/themis/pull/600)).
- All 13 supported platforms are verified on GitHub Actions, along with existing CircleCI and Bitrise tests ([#600](https://github.com/cossacklabs/themis/pull/600)).
- New Makefile targets:
- `make jsthemis` builds JsThemis from source ([#618](https://github.com/cossacklabs/themis/pull/618)).

## [0.12.0](https://github.com/cossacklabs/themis/releases/tag/0.12.0), September 27th 2019

Expand Down
5 changes: 4 additions & 1 deletion src/wrappers/themis/jsthemis/jsthemis.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ $(JSTHEMIS_OBJ)/warning_check:
endif # ifdef NPM_VERSION

$(BUILD_PATH)/jsthemis.tgz: $(JSTHEMIS_SOURCES) $(JSTHEMIS_HEADERS) $(JSTHEMIS_PACKAGE)
@cd $(BUILD_PATH) && npm pack $(abspath $(JSTHEMIS_SRC))
@cd $(BUILD_PATH) && npm pack $(abspath $(JSTHEMIS_SRC)) > /dev/null
@mv $(BUILD_PATH)/jsthemis-*.tgz $(BUILD_PATH)/jsthemis.tgz
@echo $(BUILD_PATH)/jsthemis.tgz

jsthemis: $(BUILD_PATH)/jsthemis.tgz

jsthemis_install: CMD = npm install $(abspath $(BUILD_PATH)/jsthemis.tgz)
jsthemis_install: $(BUILD_PATH)/jsthemis.tgz
Expand Down
1 change: 0 additions & 1 deletion src/wrappers/themis/jsthemis/test
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion tests/jsthemis/test.js → src/wrappers/themis/jsthemis/test/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var addon = require('jsthemis');
var addon = require('..');
Copy link
Collaborator

Choose a reason for hiding this comment

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

as I remember, the main idea of tests/<wrapper>/ folder is to have one place of all tests which allow to test installed wrapper to system. And tests inside package's folders used to test their source code and functionality.
tests/jsthemis/test.js and src/wrappers/themis/jsthemis/test/test.js have different purposes. First to check the installed package, second to test code from sources.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can get away with not having a standalone test suite, given that JsThemis normally will not be installed globally. However, if we package the tests along with the source code then it's possible to test JsThemis by going into its directory in node_modules and running npm install && npm test from there, without having to check out the repository.

It seems that npm does not provide a way to install a package and run its tests. I guess it is expected that developers do all the testing before publishing it and the users don't need to do it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should pack tests with source codes anyway. It's useful when user will install via npm and will try to change/patch something in-place and test it.

var assert = require('assert');

function expect_code(code) {
Expand Down