Fixes for test conflicts #178

k1o0 · 2019-07-15T21:03:24Z

I've fixed the conflicts between the database tests and they all pass on my laptop. Frustratingly, these very same tests fail on ZTEST. I looked at dat_test and it passes when I step through or set breakpoints (even conditional ones) but fails otherwise (but not always). I have a feeling that save is asynchronous and so on faster machines it loads the paths before the custom paths file has finished saving. I'm not sure what the best way of getting around this is. Let me know how these tests run on the rigs.

jkbhagatio · 2019-07-15T21:37:54Z

I have a feeling that save is asynchronous and so on faster machines it loads the paths before the custom paths file has finished saving

did you try throwing in a pause to test this theory? not an ideal solution, but if it works we could use it for now

k1o0 · 2019-07-16T08:05:15Z

Okay, all tests pass on ZTEST now, however there is a mysterious Alyx issue where the database sometimes returns a 500 for reasons we can't work out, however I think I've ironed out all of the client-side issues with the tests.

jkbhagatio · 2019-07-17T12:48:54Z

tests/AlyxPanel_test.m

      testCase.Panel.login('test_user', 'TapetesBloc18');
      testCase.fatalAssertTrue(testCase.Panel.AlyxInstance.IsLoggedIn,...
        'Failed to log into Alyx');
+      testCase.fatalAssertEqual(testCase.Panel.AlyxInstance.BaseURL, BaseURLTestSetup,...


@k1o0 I sent you some slack messages about this. This fails.

jkbhagatio

base url in AlyxPanel_test sometimes does not get appropriately set to 'testDev', causing some methods to fail (see slack messages)

todo · 2019-07-18T18:22:36Z

Use DELETE to test both creating new session and viewing

Rigbox/tests/AlyxPanel_test.m

Lines 233 to 237 in 829cac5

    
           % TODO Use DELETE to test both creating new session and viewing 
        
           % existing 
        
           p = testCase.Panel; 
        
           testCase.Mock.InTest = true; 
        
           testCase.Mock.UseDefaults = false;

This comment was generated by todo based on a `TODO` comment in `829cac5` in #178. cc @cortex-lab.

Undoing your change fixed this and now the url is recorded without needing a test param

k1o0 · 2019-07-18T21:03:42Z

Should be good to go

jkbhagatio · 2019-07-18T22:06:36Z

Undoing your change fixed this and now the url is recorded without needing a test param

Hm, so the issue is that BaseURL gets passed into methods in two seperate method blocks (methods (TestClassSetup), and methods (Test)) which MATLAB 2017B throws a fit over. Should we just ignore this? That means for testing on a rig I'll have to upgrade at least one training rig and one experimental rig to 2018B or newer.

k1o0 · 2019-07-19T06:02:54Z

I don't think MATLAB throws a fit. If your param is in the setup block and you try to call a test method with it the test runner just removes those tests and throws a warning as you originally pointed out. What you did to try to fix it was just wrong: the values of properties in the setup and test blocks are independent of one another. A further error was caused by issue #180. My laptop was not fast enough to encounter these errors but ZTEST was. Now all tests appear to work reliably.

jkbhagatio · 2019-07-19T07:07:45Z

I don't think MATLAB throws a fit

I just checked and this still errors on 2017B...

What you did to try to fix it was just wrong: the values of properties in the setup and test blocks are independent of one another

I don't follow...I know they're independent of one another. Can you elaborate?

k1o0 · 2019-07-19T07:12:12Z

You mean the tests as they are on this commit cause an error on 2017b? Can you open an issue an attach the command output so I can look at the error?

jkbhagatio · 2019-07-19T08:21:36Z

the issue is exactly as I've said in an above comment. Here is the output - I'm not sure it's worth an issue:

>> runtests('AlyxPanel_test')

Error using matlab.unittest.internal.TestSuiteFactory/createSuiteFromParentName (line 27)
Property BaseURL in class AlyxPanel_test must be defined as a TestParameter property for use with method
test_launchSubjectURL.

Error in testsuite>createSuite (line 137)
    suite = factory.createSuiteFromParentName(selector);

Error in testsuite (line 105)
    suites{k} = createSuite(testAsChar, selector, ...

Error in runtests (line 96)
suites = testsuite(parser.Results.tests,pvcell{:});

I guess we should always run tests on 2018B or later? And in regards to this

What you did to try to fix it was just wrong: the values of properties in the setup and test blocks are independent of one another

I did this in fact because they are independent of another, so I think you may have misunderstood my reason for doing so?

k1o0 · 2019-07-19T08:50:41Z

I removed the BaseURL from test_activeFlag (d6b7def#diff-7edecec6036e5be04c667344842875f5R490) but missed the other two methods. I didn't realize because it doesn't cause an error on 2018 however the documentation indicates that they shouldn't be accessible to the test methods. I'll push a fix for this.

Next time we Skype I'll go over how the AlyxPanel test works and why your fix failed.

k1o0 · 2019-07-19T08:57:08Z

Done in #183

jkbhagatio · 2019-07-19T09:56:54Z

I removed the BaseURL from test_activeFlag (d6b7def#diff-7edecec6036e5be04c667344842875f5R490) but missed the other two methods. I didn't realize because it doesn't cause an error on 2018 however the documentation indicates that they shouldn't be accessible to the test methods.

yeah, this is what I was looking for : )

Next time we Skype I'll go over how the AlyxPanel test works and why your fix fail

Sure, I just wanted you to be aware that I knew my "fix" wasn't proper

* Fixes for test conflicts * Ensure path is reset * Issue #180 * a fix to AlyxPanel_test/setupPanel, but still failing * Reverted some changes and added fix for activeFlag test * Bug fixes and updates for update to test db * Changes to alyx-matlab * Fix to recordWeight test * Fix for db bug cause submodule test fail

Fixes for test conflicts

004d6bd

k1o0 requested a review from jkbhagatio July 15, 2019 21:03

k1o0 added 2 commits July 16, 2019 08:11

Ensure path is reset

20cb546

Issue #180

629ec27

a fix to AlyxPanel_test/setupPanel, but still failing

539e77c

jkbhagatio reviewed Jul 17, 2019

View reviewed changes

jkbhagatio previously requested changes Jul 17, 2019

View reviewed changes

k1o0 added 2 commits July 17, 2019 19:07

Reverted some changes and added fix for activeFlag test

d6b7def

Bug fixes and updates for update to test db

829cac5

k1o0 added 3 commits July 18, 2019 21:05

Changes to alyx-matlab

3725fe9

Fix to recordWeight test

7b9aa72

Fix for db bug cause submodule test fail

f0a19d5

k1o0 requested a review from jkbhagatio July 18, 2019 21:03

jkbhagatio approved these changes Jul 19, 2019

View reviewed changes

jkbhagatio merged commit 2b6b45c into dev Jul 19, 2019

todo bot mentioned this pull request Jul 19, 2019

Use DELETE to test both creating new session and viewing #182

Open

Fixes for test conflicts #178

Fixes for test conflicts #178

Uh oh!

Conversation

k1o0 commented Jul 15, 2019

Uh oh!

jkbhagatio commented Jul 15, 2019

Uh oh!

k1o0 commented Jul 16, 2019

Uh oh!

jkbhagatio Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

jkbhagatio left a comment

Choose a reason for hiding this comment

Uh oh!

todo bot commented Jul 18, 2019

Use DELETE to test both creating new session and viewing

This comment was generated by todo based on a TODO comment in 829cac5 in #178. cc @cortex-lab.

Uh oh!

k1o0 commented Jul 18, 2019

Uh oh!

jkbhagatio commented Jul 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k1o0 commented Jul 19, 2019

Uh oh!

jkbhagatio commented Jul 19, 2019

Uh oh!

k1o0 commented Jul 19, 2019

Uh oh!

jkbhagatio commented Jul 19, 2019

Uh oh!

k1o0 commented Jul 19, 2019

Uh oh!

k1o0 commented Jul 19, 2019

Uh oh!

jkbhagatio commented Jul 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

This comment was generated by todo based on a `TODO` comment in `829cac5` in #178. cc @cortex-lab.

jkbhagatio commented Jul 18, 2019 •

edited

Loading