Disabling test until mxcubecore issues mxcubecore/#1071 is fixed

[marcus-oscarsson]

There was some nice work done to improve the tests in mxcube/mxcubecore#1066, however there is a smaller issue with the current solution, outlined here. mxcube/mxcubecore#1071.

Disabling that specific tests until that issue is fixed.

[marcus-oscarsson]

Merging this for now, @fabcor-maxiv and @dominikatrojanowska let me know if you have some input

[dominikatrojanowska]

Hi Marcus, thanks for being perceptive :)

This PR mxcube/mxcubecore#1066 was made to allow for those changes #1455, especially passing correctly this test case. Here is the uodated version

I updated also the test description to explain Max IV use case. Let me know what do you think about it. If the use case does not fits to ESRF's one I think we can keep it on our maxiv-develop branch only. Maybe let's move this part of conversation into #1455 thread.

Regarding mxcubecore: in the previous version (with idtest0 only) the test case was passing just because user with login_id = idtest1 did not exist. The credentials were incorrect thus this part was blocking login https://github.com/mxcube/mxcubecore/blob/develop/mxcubecore/HardwareObjects/mockup/ISPyBClientMockup.py#L98 Seemed it's not the case that should be tested there.

[marcus-oscarsson]

I think that the fix you did for the test case as it is here with CREDENTIALS_0 and CREDENTIALS_1 is fine and tests what you are intending.

The problem is that the fix in #1066 was only partial and allowed you to login with idtest0 and idtest1 but the same proposal is returned for both logins allowing login thus the test fails. The proposal is returned by the get_proposal method and returns the internal __test_proposal and this structure does not change with login

[dominikatrojanowska]

Should I introduce __test_proposal_0 and __test_proposal_1 or just keep the one that already exists but change some relevant fields, for instance personId?

[fabcor-maxiv]

I guess we need two different mockup proposals indeed.

Maybe there is a smart way to do it while avoiding too much copy-paste stuff, I do not know...

[dominikatrojanowska]

yes, it could be a:

• method taking user id as an argument and returning relevant dictionary eg.

def __test_proposal(self, user: Optional[str] = "") -> dict:
        proposal = None if not user else {
            "status": {"code": "ok"},
            "Person": {
                "personId": user[-1],
                "laboratoryId": 1,
                "login": None,
                "familyName": "operator on IDTESTeh1",
            },
            "Proposal": {
                "code": "idtest",
                "title": "operator on IDTESTeh1",
                "personId": user[-1],
                "number": "0",
                "proposalId": 1,
                "type": "MX",
            },
            "Session": [
                {
                    "scheduled": 0,
                    "startDate": "2013-06-11 00:00:00",
                    "endDate": "2023-06-12 07:59:59",
                    "beamlineName": self.beamline_name,
                    "timeStamp": "2013-06-11 09:40:36",
                    "comments": "Session created by the BCM",
                    "sessionId": 34591,
                    "proposalId": 1,
                    "nbShifts": 3,
                }
            ],
            "Laboratory": {"laboratoryId": 1, "name": "TEST eh1"},
        }
        return proposal

but the reading way would change to self.__test_proposal(user_id)

• or property, overwritten when the user logs in. Reading way wouldn't change (self.__test_proposal)

    @property
    def __test_proposal(self, user: Optional[str] = "") -> dict:
        return self._test_proposal

    @__test_proposal.setter
    def __test_proposal(self, user: str):
        self._test_proposal = {
            "status": {"code": "ok"},
            "Person": {
                "personId": user[-1],
                "laboratoryId": 1,
                "login": None,
                "familyName": "operator on IDTESTeh1",
            },
        ...

I dont know if anything else should be changed... in my opinion I would keep it as simple and less effort as possible since those are tests only. Tell me if I'm wrong or you have different opinion. Also some explanation of what should differ between those two proposals (corresponding to idtest0 an idtest1) would be appreciated :)

[fabcor-maxiv]

@dominikatrojanowska I think this is the wrong place to discuss this further. Maybe we should continue over there: mxcube/mxcubecore#1071.