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

Unit tests for lib.config #133

Merged
merged 3 commits into from
Jun 15, 2015
Merged

Unit tests for lib.config #133

merged 3 commits into from
Jun 15, 2015

Conversation

tonyo
Copy link
Contributor

@tonyo tonyo commented May 28, 2015

A couple of unit tests for lib.config.

def _compare_attributes(self, config, config_dict):
ntools.eq_(len(config.__dict__),
len(self.ATTRS_TO_KEYS),
"Unequal number of keys/attributes: is something missing?")
Copy link
Contributor

Choose a reason for hiding this comment

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

This can possibly by simplified to:

ntools.assert_dict_equal(config.dict, self.ATTRS_TO_KEYS)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's not really what I want. First, I compare their length, then I check that config object and config_dict dict has the same values for the corresponding attribute/key. For example I check that config.master_of_gen_key == config_dict['MasterOFGKey'].

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, ok. Nevermind then :)

@tonyo
Copy link
Contributor Author

tonyo commented May 29, 2015

Thanks for your comments, Stephen! Turns out these mocks are quite powerful if used right 👍
Hope it looks OK now.

@pszal
Copy link
Contributor

pszal commented May 29, 2015

@rev112 lgtm and you can merge... but you can wait for @kormat as well, I'm sure he will be happy to review ;-)

@tonyo
Copy link
Contributor Author

tonyo commented May 29, 2015

No way! I am not merging anything like this without @kormat.

@kormat
Copy link
Contributor

kormat commented May 29, 2015

(Just a quick note that i am on holidays until next thursday, sorry for the delay :)

def test_basic(self):
cfg = Config()
cfg.parse_dict(self.config_json)
self._compare_attributes(cfg, self.config_json)
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is now the only place that _compare_attributes is called from, it probably makes sense to just move it into this test class.

@kormat
Copy link
Contributor

kormat commented Jun 15, 2015

One small comment, otherwise LGTM :)

@tonyo
Copy link
Contributor Author

tonyo commented Jun 15, 2015

Updated!

tonyo added a commit that referenced this pull request Jun 15, 2015
@tonyo tonyo merged commit 7bf5871 into scionproto:master Jun 15, 2015
@tonyo tonyo deleted the config_unittests branch June 15, 2015 20:23
@kormat
Copy link
Contributor

kormat commented Jun 16, 2015

(For #129)

@kormat kormat mentioned this pull request Jun 17, 2015
FR4NK-W pushed a commit to FR4NK-W/osourced-scion that referenced this pull request Feb 14, 2017
JordiSubira added a commit to JordiSubira/scion that referenced this pull request Nov 15, 2022
* add convenience method to serrors.List

* WIP keeper and manager restructuring

* WIP keeper and manager restructuring

* WIP keeper and manager restructuring

* WIP keeper and manager restructuring

* WIP fix keeper tests

* recreate mocks

* WIP remove unused code

* fix keeper tests

* manager can delete expired indices

* delete expired indices on startup

* fix bug removing active indices

* add test checking removing active indices

* manager waits until operator ready

* rename store test to performance test

* reenable basic store test

* Fix two bugs in initiator.

1. The keeper did not set the index of the local reservation to the
appropriate state before (or after) calling the store to confirm or
activate indices.
2. The keeper didn't access its local entries with a pointer, but value
copy.

* Make seg.Index.state public

* When manager or store fail, rollback all changes.

Using the simplified State and SetIndexInactive, from the manager or
keeper, ensure that all local indices and reservations are rolled back
to the original state before the store or manager failed.

* add empty store test

* Fix steps if down path. RawPath --> TransportPath.

When dealing with a down path SegR, the steps are always stored in the direction
of the reservation.
The steps are expected in the direction of the control plane messaging for
init confirm and init activate index.
The manager and keeper are in charge to revert the steps whenever
initializing/triggering a reservation or index.
Rename RawPath to TransportPath in SegR and its setup request.

* simplify some code

* Do not log message for persistent quic when idle.

When the persistent quic client tries to reuse a session, it might
get an error of the net.Error type, with Timetout true. In this case,
just ignore the error and don't print anything.

* Add hop fields at the initiator AS.

Also compute the colibri path on activation.

* Remove duplicated Ingress,Egress fields from SegR.

Update tests.

* SegRs store only colibri transport paths.

Initialization doesn't need a path, unless it is colibri.
Store accepts only colibri paths, or nil.
Don't create steps from snet paths.
Remove panic() from tests.
Adapt tests.

* Transport path validation happens only if colibri path.

* cleanup

* cleanup rest of slayerspath.Path wrong uses in colibri

* Fix some bugs/quirks.

Reverse a nil path returns nil.
Restore 0 bytes returns nil path.
Client operator uses colibri if present.

* Temporarily patch BUG involving colibri transport.

When contacting the next colibri service using a colibri path,
at a stitching point, the RPC seems to never arrive at the
destination.
Patch for now using nil, which triggers the operator to use a
regular scion path.

* Review use of rawPath in colibri code.

Whenever it is the transport, rename it to transportPath.

* refactor manager to serviceProvider interface

* rename to ServiceFacilitator

* Fixes from review.

Simplified activateIndex in keeper.
Fixed down-path reservations not initializable by core ASes.
CurrentStep is always modified in the service.
Fix some comments.
Simplify some if-else conditions.
Move SnetToDataplanePath to snet/path.

* from review

* split DeriveColibriPath()

* adapt logic DeriveColibriPathAtDestination

Co-authored-by: Jordi Subirà Nieto <jordi.subira.nieto@gmail.com>
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.

None yet

3 participants