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.

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

Disabling privacy for testing #1882

Closed
sigmundch opened this issue Feb 27, 2012 · 25 comments
Closed

Disabling privacy for testing #1882

sigmundch opened this issue Feb 27, 2012 · 25 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-obsolete Closed as the reported issue is no longer relevant P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@sigmundch
Copy link
Member

sigmundch commented Feb 27, 2012

It would be very useful to have some mechanism to test private members without having to expose them in a library.

One option is to add a flag to allow access to private members, for instance:
'--disable_privacy'

The semantics could be either:
(a) all imports now export all names (including private names) to the imported library.
(b) only the imports of the initial script have this semantics (so tests can see internals of the libraries it imports, but the rest of the code wont have to worry about name collisions).

This requires changes in both the VM and frog. I'm arbitrarily labeling this bug with the frog area because I can't mark both.

This would be a test case that should work:
mysource.dart:
  #library("mysource");
  get _fooForTesting() => 1;

mytest.dart:
  #import("mysource.dart");
  main() => Expect.equals(1, _fooForTesting);


dart --disable_privacy mytest.dart should pass.

@anders-sandholm
Copy link
Contributor

Removed Area-Frog label.
Added Area-Dart2JS, FromAreaFrog labels.

@kasperl
Copy link

kasperl commented Jun 12, 2012

Removed Type-Defect, Priority-Medium, FromAreaFrog labels.
Added Type-Enhancement, Priority-Low labels.

@kasperl
Copy link

kasperl commented Sep 3, 2012

Added this to the Later milestone.

@kasperl
Copy link

kasperl commented Oct 17, 2012

Removed this from the Later milestone.

@kasperl
Copy link

kasperl commented Oct 17, 2012

Added this to the Later milestone.

@kasperl
Copy link

kasperl commented Oct 17, 2012

Removed Priority-Low label.
Added Priority-Medium label.

@kasperl
Copy link

kasperl commented May 23, 2013

Added TriageForM5 label.

@kasperl
Copy link

kasperl commented May 28, 2013

Removed TriageForM5 label.

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@floitschG
Copy link
Contributor

Reassigning to language.
I fully agree that a way to access private state during testing would be extremely useful.


Removed Area-Dart2JS label.
Added Area-Language label.

@gbracha
Copy link
Contributor

gbracha commented Jan 3, 2015

Issue #10219 has been merged into this issue.

@sigmundch sigmundch added Type-Enhancement area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). labels Jan 3, 2015
@rmacnak-google
Copy link
Contributor

Mirrors can be used to bypass library privacy.

@floitschG
Copy link
Contributor

That doesn't work on Flutter, dart2js, ... and is extremely painful.

@floitschG floitschG reopened this Dec 5, 2015
@eernstg
Copy link
Member

eernstg commented Dec 7, 2015

For reflectable, a programmatic privacy override mechanism would be very useful as well.

@michaelcarter-wf
Copy link

+1 for a way to turn off privacy in tests.

@jtmcdole
Copy link
Contributor

import 'dart:mirrors';

class Foo {
int _privateInt = 12;
}
...
var inst = reflect(new Foo());
print(inst.getField(#_privateInt).reflectee); // 12

@jtmcdole
Copy link
Contributor

Scratch that; doesn't work across libraries.

+1 for a way to turn off privacy :)

@zoechi
Copy link
Contributor

zoechi commented Jan 21, 2016

It was mentioned in discussions to introduce something like friend libraries which are allowed to access each others private members also as a possible replacement for part / part of. I'm not sure how this would work out for all use cases.
Anyway I also think there should be a way around this limitation.

@lrhn
Copy link
Member

lrhn commented Jan 21, 2016

Just "turning off" privacy is not going to work. That would make "_foo" in two different libraries suddenly conflict or override each other, so some code will just fail to work - or even compile. Private names is a way to avoid having to invent unique names as much as it's a way of restricting access.

I highly recommend not using privacy for things you want to test. Instead move them to a separate helper library where they are public, and run tests against that library. Then export only the parts you need in the real library.
That won't work for private members of public classes, but if you assume the presence of private fields or methods, then your tests are not refactoring-safe. Changing the name of a private member should be a completely invisible change. Testing the public methods should be sufficient to test that a class actually behaves as it should. If you have functionality that you want to test independently of the class API, then that functionality is likely a good choice for moving into static helper functions that can actually be tested independently.

(In other words: If you think you need access to private stuff, consider redesigning your class and library layout to not need it any more).

@rmacnak-google
Copy link
Contributor

@jtmcdole To do it across libraries, you need to get the private symbol with respect to the library you're trying to access.

class Foo {
int _privateInt = 12;
}
...
var inst = reflect(new Foo());
var sym = MirrorSystem.getSymbol('_privateInt', inst.type.owner);
print(inst.getField(sym).reflectee); // 12

@jtmcdole
Copy link
Contributor

@rmacnak-google, yep, I found that out via a different method and it isn't too messy. I like the type.owner though. Just wish there was a ways to still use #_privateInt in that expression.

@lrhn
Copy link
Member

lrhn commented Jan 25, 2016

You can't use #_privateInt because it's the private name belonging to your library. It's a completely different name than one with similar syntax in a different library.

@kevmoo kevmoo removed the triaged label Feb 29, 2016
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed triaged labels Feb 29, 2016
@zoechi
Copy link
Contributor

zoechi commented Mar 19, 2016

Related #22841

@munificent munificent changed the title disabling privacy for testing Disabling privacy for testing Dec 19, 2016
@zoechi
Copy link
Contributor

zoechi commented Feb 23, 2017

#28273 seems to be the chosen approach. Can this issue be closed?

@matanlurey matanlurey added the closed-obsolete Closed as the reported issue is no longer relevant label Jun 19, 2018
copybara-service bot pushed a commit that referenced this issue Jan 17, 2023
…, webdev

Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/c4ab682..a99abd4):
  a99abd4b  2023-01-16  dependabot[bot]  Bump dart-lang/setup-dart from 1.3 to 1.4 (#3305)
  a692eeaa  2023-01-16  dependabot[bot]  Bump github/codeql-action from 2.1.37 to 2.1.38 (#3304)
  a43a6c2b  2023-01-14  Parker Lougheed  Remove search code debug prints (#3300)
  cf28572a  2023-01-14  Parker Lougheed  Remove obsolete doc_packages tool (#3301)
  fcbadcd7  2023-01-14  Parker Lougheed  Remove null safety badge (#3295)
  263ea617  2023-01-14  Parker Lougheed  Use spread syntax instead of add (#3296)
  820b5ba5  2023-01-14  Parker Lougheed  Use equal signs to set default parameter values (#3298)
  31e9c797  2023-01-14  Parker Lougheed  Fix build by removing test opting out of null safety (#3297)
  d4495c2c  2023-01-11  dependabot[bot]  Bump actions/checkout from 3.2.0 to 3.3.0 (#3292)
  3ae8eef5  2023-01-11  dependabot[bot]  Bump actions/upload-artifact from 3.1.1 to 3.1.2 (#3291)
  8a9e4691  2023-01-11  dependabot[bot]  Bump actions/cache from 3.2.2 to 3.2.3 (#3290)

http (https://github.com/dart-lang/http/compare/d434d42..c955c7e):
  c955c7e  2023-01-13  Brian Quinlan  Add consistent implementations for `close`. (#851)

intl (https://github.com/dart-lang/intl/compare/c61fdd1..6140b60):
  6140b60  2023-01-12  Googler  Internal change

mime (https://github.com/dart-lang/mime/compare/273d454..034471a):
  034471a  2023-01-11  Kevin Moore  Prepare to release v1.0.4 (#80)

string_scanner (https://github.com/dart-lang/string_scanner/compare/c58618d..0454980):
  0454980  2023-01-17  Kevin Moore  dependabot: monthly is plenty (#54)

sync_http (https://github.com/dart-lang/sync_http/compare/8622614..36a1bd0):
  36a1bd0  2023-01-11  Kevin Moore  Bump min SDK, enable and fix new lints (#34)

test (https://github.com/dart-lang/test/compare/932a652..43fd928):
  43fd9284  2023-01-17  Jacob MacDonald  delete some old integration test helper files that were opted out (#1850)
  2c59fb6c  2023-01-17  Kevin Moore  Run no response daily (#1849)
  8ea50552  2023-01-12  joshualitt  Update wasm integration test to use generated JS runtime for Dart2Wasm. (#1844)
  9a23b72a  2023-01-11  Nate Bosch  Prepare to publish (#1843)
  d887825a  2023-01-11  Derek Xu  Update vm_service constraints to >=6.0.0 <11.0.0 (#1842)

webdev (https://github.com/dart-lang/webdev/compare/094ee97..f978b90):
  f978b90  2023-01-13  Elliott Brooks (she/her)  [MV3] Debug session persists across closing and opening Chrome DevTools (#1894)
  b1b4eff  2023-01-13  Anna Gringauze  Prepare for dart 3.0 alpha changes: generate assets (#1887)
  969f41f  2023-01-13  Elliott Brooks (she/her)  Save encoded URI for ACX DevTools (#1890)
  8384a11  2023-01-13  Elliott Brooks (she/her)  Skip flaky test on windows (#1893)
  8224045  2023-01-13  Elliott Brooks (she/her)  Ignore `illegal_language_version_override` for non null-safe fixtures (#1891)
  e42a030  2023-01-13  Elliott Brooks (she/her)  Re-enable most test cases in `devtools_test` (#1881)
  e134e5b  2023-01-11  Elliott Brooks (she/her)  [MV3] Dart debug extension supports DWDS versions < `17.0.0` (#1882)
  ed80c94  2023-01-11  Elliott Brooks (she/her)  [MV3] Prepare extension for release (#1886)
  be616cd  2023-01-10  Anna Gringauze  Return error from expression evaluation if the evaluator is closed. (#1884)
  18b3277  2023-01-10  Elliott Brooks (she/her)  [MV3] Fix late initialization error on debugger detach (#1879)
  03d4035  2023-01-10  Elliott Brooks (she/her)  Check if storage object exists before trying to read properties(#1883)
  ae55fec  2023-01-10  Elliott Brooks (she/her)  Format manifest.json (#1885)
  3743293  2023-01-10  Anna Gringauze  Fix race condition on simultaneous hot restarts (#1870)

Change-Id: I2bddd015f1e054eb9e24afb247f9c470257560a9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279073
Auto-Submit: Devon Carew <devoncarew@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-obsolete Closed as the reported issue is no longer relevant P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests