#6221 closed change (fixed)

[emscripten] Setup native test environment (gtest) for emscripten C++ code.

Reported by: sergz Assignee: hfiguiere
Priority: P2 Milestone:
Module: Core Keywords: emscripten
Cc: hfiguiere Blocked By: #6241
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29680720/

Description (last modified by kzar)

Background

We'd like to setup a native test environment to test the C++ code which is making its way into adblockpluscore. It makes sense to test the C++ code before it's been converted into JavaScript for the following reasons:

  1. Emscripten C++ code is going to be used as C++ code in native applications (derivatives based on libadblockplus) and thus should be tested in the native tests.
  2. There is functionality in the C++ code which is never directly exposed via JavaScript. For example utility functionality, such as String and various Map classes, which should be tested.
  3. It's very hard to debug emscripten generated code, so these tests should simplify debugging of the C++ part.

What to change

  • add google test framework as a dependency
  • create a minimal project with the symbolic test(s)
  • update the test running scripts so that the native tests are run with the JavaScript unit tests and browser tests
  • if #6220 is already landed then add corresponding changes in the CI configurations
  • create following issues for the tests of the rest functionality.

Change History (8)

comment:1 Changed 23 months ago by kzar

  • Cc kzar removed
  • Description modified (diff)
  • Keywords emscripten added
  • Priority changed from Unknown to P2
  • Ready set
  • Summary changed from [emscripten] Add native tests (gtest) for emscripten C++ code. to [emscripten] Setup native test environment (gtest) for emscripten C++ code.

Sounds sensible to me, I've tidied up the issue a little bit and marked it as ready. Removing myself from Cc since C++ and Emscripten is not something I know much about.

comment:2 follow-up: Changed 23 months ago by hfiguiere

FWIW I started a patch to build the emscripten code natively. Including some test.

comment:3 Changed 22 months ago by hfiguiere

  • Blocked By 6241 added

comment:4 in reply to: ↑ 2 Changed 22 months ago by hfiguiere

Replying to hfiguiere:

FWIW I started a patch to build the emscripten code natively. Including some test.

File issue #6241 for this.

comment:5 Changed 22 months ago by hfiguiere

  • Owner set to hfiguiere

comment:6 Changed 22 months ago by hfiguiere

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:7 Changed 22 months ago by abpbot

A commit referencing this issue has landed:
Issue 6221 - Add native tests.

comment:8 Changed 22 months ago by hfiguiere

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.